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@xxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
