linux-kernel - Re: [patch 05/27] fs: brlock vfsmount

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 26 Apr 2009 08:36:54 +0200
From:	Nick Piggin <npiggin@...e.de>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [patch 05/27] fs: brlock vfsmount_lock

On Sat, Apr 25, 2009 at 04:50:40AM +0100, Al Viro wrote:
> On Sat, Apr 25, 2009 at 11:20:25AM +1000, npiggin@...e.de wrote:
> 
> [overall: sane idea, but...]
> 
> > +void vfsmount_read_lock(void)
> > +{
> > +	spinlock_t *lock;
> > +
> > +	lock = &get_cpu_var(vfsmount_lock);
> > +	spin_lock(lock);
> > +}
> > +
> > +void vfsmount_read_unlock(void)
> > +{
> > +	spinlock_t *lock;
> > +
> > +	lock = &__get_cpu_var(vfsmount_lock);
> > +	spin_unlock(lock);
> > +	put_cpu_var(vfsmount_lock);
> > +}
> 
> These might be hot enough to be worth inlining, at least in fs/namei.c
> users.  Or not - really needs testing.

Hmm, no you could be right. Most of the code is still OOL in the
spinlock call, so avoiding one level of call chain is probably
going to be a win. I'll see how much it increases code size.

 
> > @@ -68,9 +113,9 @@ static int mnt_alloc_id(struct vfsmount
> >  
> >  retry:
> >  	ida_pre_get(&mnt_id_ida, GFP_KERNEL);
> > -	spin_lock(&vfsmount_lock);
> > +	vfsmount_write_lock();
> >  	res = ida_get_new(&mnt_id_ida, &mnt->mnt_id);
> > -	spin_unlock(&vfsmount_lock);
> > +	vfsmount_write_unlock();
> 
> Yuck.  _Really_ an overkill here.
> 
> >  static void mnt_free_id(struct vfsmount *mnt)
> >  {
> > -	spin_lock(&vfsmount_lock);
> > +	vfsmount_write_lock();
> >  	ida_remove(&mnt_id_ida, mnt->mnt_id);
> > -	spin_unlock(&vfsmount_lock);
> > +	vfsmount_write_unlock();
> >  }
> 
> Ditto.

Yeah, wanted to try going as simple as possible for the first cut.
Shall I just add another spinlock for it?

> Missing: description of when we need it for read/when we need it for write.

OK, I'll work on the documentation.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/