[RFC][PATCH 12/16] writeback: add to bdi_list in the forker thread

Previous thread: [RFC][PATCH 15/16] writeback: clean-up the warning about non-registered bdi by Artem Bityutskiy on Friday, July 16, 2010 - 5:45 am. (2 messages)

Next thread: [RFC][PATCH 09/16] writeback: do not lose wake-ups in bdi threads by Artem Bityutskiy on Friday, July 16, 2010 - 5:45 am. (2 messages)
From: Artem Bityutskiy
Date: Friday, July 16, 2010 - 5:45 am

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Before creating a bdi thread, the forker thread first removes the
corresponding bdi from the 'bdi_list', then creates the bdi thread
and wakes it up. The thread then adds itself back to the 'bdi_list'.

There is no problem with this, except that it makes the logic a
tiny bit more twisted, because the code reader has to spend some
time to figure out when the bdi is moved back. But this is minor.

However, this patch makes the forker thread add the bdi back to
the 'bdi_list' itself, rather than letting the bdi thread do this.

The reason of the change is to prepare for further changes. Namely,
the further patches will move the bdi thread exiting logic from
bdi threads to the forker thread. So the forker thread will kill
inactive bdi threads. And for the killing case, the forker thread
will have to add bdi's back to to the 'bdi_list' itself. And to make
the code consistent, it is better if we use the same approach for
the bdi thread creation path as well. This is just more elegant.

Note, bdi threads added themselves to the head of the 'bdi_list',
but in the error path the forker thread added them to the tail of
the 'bdi_list'. This patch modifies the code so that bdi's are
always added to the tail. There is no fundamental reason for this,
this is again, just for consistency and to make the code simpler.
I just do not see any problem adding them back to the tail instead
of the head. And this should even be a bit better, because the
next iteration of the bdi forker thread loop will start looking
to the 'bdi_list' from the head, and it will find a bdi which
requires attention a bit faster.

And for absolutely the same reasons, this patch also moves the
piece of code which clears the BDI_pending bit and wakes up the
waiters from bdi threads to the forker thread.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 fs/fs-writeback.c |   14 --------------
 mm/backing-dev.c  |   21 +++++++++++++++------
 2 files ...
From: Christoph Hellwig
Date: Saturday, July 17, 2010 - 11:58 pm

What about never removing a bdi from bdi_list?  If we have the
correct checks for dirty_io and the task there's no need to
ever remove a life bdi from the list.  Just add it in bdi_register
and remove it in bdi_unregister.

--

From: Artem Bityutskiy
Date: Tuesday, July 20, 2010 - 4:07 am

Good idea, thanks, indeed I do not see why we need removing it.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

--

From: Artem Bityutskiy
Date: Tuesday, July 20, 2010 - 4:32 am

But I think it will be less error-prone and nicer to still have this
patch, then move the killing logic from bdi threads to the forker task,
and then, as a separate patch on top of that, get rid of this removing
bdi from bdi_list part. I mean, this way the patch series will be more
logical and finer grained and easier to review.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

--

Previous thread: [RFC][PATCH 15/16] writeback: clean-up the warning about non-registered bdi by Artem Bityutskiy on Friday, July 16, 2010 - 5:45 am. (2 messages)

Next thread: [RFC][PATCH 09/16] writeback: do not lose wake-ups in bdi threads by Artem Bityutskiy on Friday, July 16, 2010 - 5:45 am. (2 messages)