Re: CVS commit: src/sys/kern

[Rin Okuyama <rokuyama.rk%gmail.com@localhost>]

Hmm, I was still confused.

On 2025/04/10 11:08, Rin Okuyama wrote:
> On 2025/04/09 14:38, Rin Okuyama wrote:
> > Module Name:    src
> > Committed By:    rin
> > Date:        Wed Apr  9 05:38:01 UTC 2025
> >
> > Modified Files:
> >     src/sys/kern: subr_log.c
> >
> > Log Message:
> > logread: Stop reading msgbuf without log_lock being held
>
> Oops, sorry, here I made typo; s/reading/writing/

This is still not correct. The problem is **read** access to
**log** without log_lock being held.

Write access to msgbuf is done without lock **both** before
and after the change.

My bad, sorry for bothering you many times.

rin

> Thanks,
> rin
>
> > Idea taken from FreeBSD.
> >
> > XXX
> > The size of temporal buffer on stack (== 128) is also taken from FreeBSD.
> > We are not very sure whether this value is optimal or not. But we don't
> > expect performance regression from this choice.
> >
> > No new regression for full ATF run on amd64 with DIAGNOSTIC+LOCKDEBUG
> > kernel and rump kernel.
> >
> > Authored by knakahara@.
> >
> >
> > To generate a diff of this commit:
> > cvs rdiff -u -r1.65 -r1.66 src/sys/kern/subr_log.c
> >
> > Please note that diffs are not public domain; they are subject to the
> > copyright notices on the relevant files.
> >
> >
> > Modified files:
> >
> > Index: src/sys/kern/subr_log.c
> > diff -u src/sys/kern/subr_log.c:1.65 src/sys/kern/subr_log.c:1.66
> > --- src/sys/kern/subr_log.c:1.65    Tue Apr  8 04:19:28 2025
> > +++ src/sys/kern/subr_log.c    Wed Apr  9 05:38:01 2025
> > @@ -1,4 +1,4 @@
> > -/*    $NetBSD: subr_log.c,v 1.65 2025/04/08 04:19:28 rin Exp $    */
> > +/*    $NetBSD: subr_log.c,v 1.66 2025/04/09 05:38:01 rin Exp $    */
> >   /*-
> >    * Copyright (c) 2007, 2008 The NetBSD Foundation, Inc.
> > @@ -65,7 +65,7 @@
> >    */
> >   #include <sys/cdefs.h>
> > -__KERNEL_RCSID(0, "$NetBSD: subr_log.c,v 1.65 2025/04/08 04:19:28 rin 
> > Exp $");
> > +__KERNEL_RCSID(0, "$NetBSD: subr_log.c,v 1.66 2025/04/09 05:38:01 rin 
> > Exp $");
> >   #include <sys/param.h>
> >   #include <sys/systm.h>
> > @@ -219,20 +219,25 @@ logread(dev_t dev, struct uio *uio, int
> >           }
> >       }
> >       while (uio->uio_resid > 0) {
> > +        char buf[128]; /* taken from FreeBSD */
> > +
> >           l = mbp->msg_bufx - mbp->msg_bufr;
> >           if (l < 0)
> >               l = mbp->msg_bufs - mbp->msg_bufr;
> >           l = ulmin(l, uio->uio_resid);
> >           if (l == 0)
> >               break;
> > +
> > +        l = ulmin(l, sizeof(buf));
> > +        memcpy(buf, &mbp->msg_bufc[mbp->msg_bufr], l);
> > +        mbp->msg_bufr += l;
> > +        if (mbp->msg_bufr < 0 || mbp->msg_bufr >= mbp->msg_bufs)
> > +            mbp->msg_bufr = 0;
> >           mutex_spin_exit(&log_lock);
> > -        error = uiomove(&mbp->msg_bufc[mbp->msg_bufr], l, uio);
> > +        error = uiomove(buf, l, uio);
> >           mutex_spin_enter(&log_lock);
> >           if (error)
> >               break;
> > -        mbp->msg_bufr += l;
> > -        if (mbp->msg_bufr < 0 || mbp->msg_bufr >= mbp->msg_bufs)
> > -            mbp->msg_bufr = 0;
> >       }
> >       mutex_spin_exit(&log_lock);