On (02/28/11 16:59), Andreas Bießmann wrote:
> Dear Sergey Senozhatsky,
>
> Am 28.02.2011 16:43, schrieb Sergey Senozhatsky:
> > On (02/28/11 16:25), Andreas Bießmann wrote:
>
> >> The reference to sb->s_bdi may be deleted from mmc_blk_remove() ->
> >> del_gendisk() -> unlink_gendisk() -> bdi_unregister() -> bdi_prune_sb() while
> >> another instance try to write some data to the device.
> >>
> >> Signed-off-by: Andreas Bießmann <biessmann@xxxxxxxxxxxxx>
> >> ---
> >> fs/fs-writeback.c | 3 +++
> >> 1 files changed, 3 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> >> index cdbf7ac..96b4b25 100644
> >> --- a/fs/fs-writeback.c
> >> +++ b/fs/fs-writeback.c
> >> @@ -1047,6 +1047,9 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> >> if (!was_dirty) {
> >> bdi = inode_to_bdi(inode);
> >>
> >> + if (!bdi)
> >> + goto out;
> >> +
> >> if (bdi_cap_writeback_dirty(bdi)) {
> >> WARN(!test_bit(BDI_registered, &bdi->state),
> >> "bdi-%s not registered\n", bdi->name);
> >
> > Hello,
> > I had something very similar to this some time ago
> > https://lkml.org/lkml/2010/12/9/436
>
> Sorry, I did not see that patch.
>
No problem :-)
> > However, I'm not sure that this check is sufficient.
>
> Why are you think this is not sufficient?
> If an instance try to write that specific inode to an physical device
> which is not longer available how should we react then?
>
I think the path which `kills' the device should take care of that.
Otherwise, IMHO, we have:
- ok, we're falling on line 42 -- let's fix line 42
ignoring the fact that there are reasons which led to faulty line 42,
which are, for example:
#0
604 spin_lock(&sb_lock);
605 list_for_each_entry(sb, &super_blocks, s_list) {
606 if (sb->s_bdi == bdi)
607 sb->s_bdi = NULL;
608 }
609 spin_unlock(&sb_lock);
#1 bdi_prune_sb
#2 bdi_unregister
#3 del_gendisk
Of course, I may be wrong.
> Another solution could be to clean up all instances referring to that
> superblock in del_/unlink_gendisk(). But I think to check the return of
> inode_to_bdi() is needed in any case.
>
Sergey
Attachment:
pgpZWEHbNm0sB.pgp
Description: PGP signature
