lkml.org 
[lkml]   [2021]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH -next] NFSD: Use DEFINE_SPINLOCK() for spinlock
    On Tue, Apr 06, 2021 at 03:46:34PM +0000, Chuck Lever III wrote:
    >
    >
    > > On Apr 6, 2021, at 8:08 AM, Huang Guobin <huangguobin4@huawei.com> wrote:
    > >
    > > From: Guobin Huang <huangguobin4@huawei.com>
    > >
    > > spinlock can be initialized automatically with DEFINE_SPINLOCK()
    > > rather than explicitly calling spin_lock_init().
    > >
    > > Reported-by: Hulk Robot <hulkci@huawei.com>
    > > Signed-off-by: Guobin Huang <huangguobin4@huawei.com>
    >
    > This same patch was sent last July, without the Reported-by:
    >
    > https://lore.kernel.org/linux-nfs/1617710898-49064-1-git-send-email-huangguobin4@huawei.com/T/#u
    >
    > If I'm reading the code correctly, it appears that set_max_drc()
    > can be called more than once by user space API functions via
    > nfsd_create_serv().
    >
    > It seems to me we want to guarantee that nfsd_drc_lock is
    > initialized exactly once, and using DEFINE_SPINLOCK() would
    > do just that.
    >
    > Likewise, re-initializing nfsd_drc_mem_used is probably not good
    > to do either, but clobbering a spinlock like that might lead to
    > a crash.
    >
    > Bruce, any further thoughts?

    The other nfsd_drc_lock users run in nfsd threads, which shouldn't be
    running yet at the point we call set_max_drc.

    Reinitializing the lock each time is pretty weird, though, ACK to the
    patch even if it's just cleanup.

    --b.

    >
    >
    > > ---
    > > fs/nfsd/nfssvc.c | 3 +--
    > > 1 file changed, 1 insertion(+), 2 deletions(-)
    > >
    > > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
    > > index b2eef4112bc2..82ba034fa579 100644
    > > --- a/fs/nfsd/nfssvc.c
    > > +++ b/fs/nfsd/nfssvc.c
    > > @@ -84,7 +84,7 @@ DEFINE_MUTEX(nfsd_mutex);
    > > * version 4.1 DRC caches.
    > > * nfsd_drc_pages_used tracks the current version 4.1 DRC memory usage.
    > > */
    > > -spinlock_t nfsd_drc_lock;
    > > +DEFINE_SPINLOCK(nfsd_drc_lock);
    > > unsigned long nfsd_drc_max_mem;
    > > unsigned long nfsd_drc_mem_used;
    > >
    > > @@ -563,7 +563,6 @@ static void set_max_drc(void)
    > > nfsd_drc_max_mem = (nr_free_buffer_pages()
    > > >> NFSD_DRC_SIZE_SHIFT) * PAGE_SIZE;
    > > nfsd_drc_mem_used = 0;
    > > - spin_lock_init(&nfsd_drc_lock);
    > > dprintk("%s nfsd_drc_max_mem %lu \n", __func__, nfsd_drc_max_mem);
    > > }
    > >
    > >
    >
    > --
    > Chuck Lever
    >
    >

    \
     
     \ /
      Last update: 2021-04-06 17:55    [W:5.067 / U:0.668 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site