Re: [PATCH 5/9] writeback: support > 1 flusher thread per bdi

[Jens Axboe]

On Mon, Aug 24 2009, Jan Kara wrote:
> 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 ;).

I don't plan to merge it before we have that sorted, don't worry. I'll
be posting it soon here again!

-- 
Jens Axboe

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