linux-kernel - Re: [PATCH 5/9] writeback: support

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: <20090824123611.GE10080@duck.novell.com>
Date:	Mon, 24 Aug 2009 14:36:11 +0200
From:	Jan Kara <jack@...e.cz>
To:	Jens Axboe <jens.axboe@...cle.com>
Cc:	Jan Kara <jack@...e.cz>, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, chris.mason@...cle.com,
	david@...morbit.com, hch@...radead.org, akpm@...ux-foundation.org,
	yanmin_zhang@...ux.intel.com, richard@....demon.co.uk,
	damien.wyart@...e.fr, fweisbec@...il.com, Alan.Brunelle@...com
Subject: Re: [PATCH 5/9] writeback: support > 1 flusher thread per bdi

On Mon 24-08-09 13:43:26, Jens Axboe wrote:
> On Thu, Aug 06 2009, Jan Kara wrote:
> >   Actually, looking again that the work struct "state" field has lots of
> > free bits. I think the code looks nicer with the attached patch, what do
> > you think?
> > 
> > > >   2) I'd introduce a flag with the meaning: free the work when you are
> > > > done. Obviusly this flag makes sence only with dynamically allocated work
> > > > structure. There would be no "on stack" flag.
> > > >   3) I'd create a function:
> > > > bdi_wait_work_submitted()
> > > >   which you'd have to call whenever you didn't set the flag and want to
> > > > free the work (either explicitely, or via returning from a function which
> > > > has the structure on stack).
> > > >   It would do:
> > > > bdi_wait_on_work_clear(work);
> > > > call_rcu(&work->rcu_head, bdi_work_free);
> > > > 
> > > >   wb_work_complete() would just depending on the flag setting either
> > > > completely do away with the work struct or just do bdi_work_clear().
> > > > 
> > > >   IMO that would make the code easier to check and also less prone to
> > > > errors (currently you have to think twice when you have to wait for the rcu
> > > > period, call bdi_work_free, etc.).
> > > 
> > > Didn't we go over all that last time, too?
> >   Well, probably about something similar. But this time I have a patch ;-)
> > Compile tested only... IMO it looks nicer this way as it wraps up all the
> > details of work freeing into one function.
> 
> The first patch looks nice and obvious, I'll fold that in with the
> original patch if you don't mind. It's definitely cleaner, instead of
> overloading the pointer.
  Yes, that's fine.

> The second one I'd rather hold off on, I've run over the existing code
> many times and tested it heavily threaded and know it's safe. So I'd
> rather not introduce any drastic changes there so close to 2.6.32, but
> I'd be happy to revisit this soon after merge. OK?
  Fine with me, I'm just not sure about the merging in 2.6.32 - the
umount_sem and sb->s_count problems (http://lkml.org/lkml/2009/8/5/322 -
BTW I didn't see a response from you) should get sorted out before the
merge. To be honest, I'm not much in favor of merging your patches before
having resolved that and I think Christoph Hellwig or Al Viro will express
their opinion even more strongly ;).

								Honza
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
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/