linux-kernel - Re: [PATCHv6 07/16] pps: move idr stuff to pps.c

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]

Message-Id: <20101217171829.814a6be3.akpm@linux-foundation.org>
Date:	Fri, 17 Dec 2010 17:18:29 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Alexander Gordeev <lasaine@....cs.msu.su>
Cc:	linux-kernel@...r.kernel.org,
	"Nikita V\. Youshchenko" <yoush@...msu.su>,
	linuxpps@...enneenne.com, Rodolfo Giometti <giometti@...eenne.com>
Subject: Re: [PATCHv6 07/16] pps: move idr stuff to pps.c

On Sat, 18 Dec 2010 04:07:38 +0300
Alexander Gordeev <lasaine@....cs.msu.su> wrote:

> > > +	if (err < 0)
> > > +		return err;
> > 
> > The IDR interface really sucks :(
> > 
> > What this code should be doing is
> > 
> > retry:
> > 	if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0)
> > 		return -ENOMEM;
> > 	spin_lock_irq(&pps_idr_lock);
> > 	err = idr_get_new(&pps_idr, pps, &pps->id);
> > 	spin_unlock_irq(&pps_idr_lock);
> > 	if (err < 0) {
> > 		if (err == -EAGAIN)
> > 			goto retry;
> > 		return err;
> > 	}
> > 
> > this way it correctly handles the case where the idr_pre_get()
> > succeeded in precharging the pool, but some other task cam in and stole
> > your reservation.
> 
> Yeah, I see. Maybe switching from spin lock to mutex and protecting the
> whole thing with it can do? Like this:
> 
> ...
> mutex_lock(&pps_idr_lock);
> if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0) {
> 	mutex_unlock(&pps_idr_lock);
> 	return -ENOMEM;
> }
> err = idr_get_new(&pps_idr, pps, &pps->id);
> mutex_unlock(&pps_idr_lock);
> 
> if (err < 0)
> 	return err;
> ...

That works so, as long as no code path will take pps_idr_lock in the
page allocator direct-reclaim path.

That's unlikely to be happening in the PPS driver of course.  It's
conceivable that some filesystems might want to read the time when
playing with file timestamps in the direct-reclaim path, but I assume
pps_idr_lock wouldn't be taken on any read-system-time paths.

--
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/