[GIT PULL] UDF tree fixes

[GIT PULL] UDF tree fixes

[Jan Kara]

  Hi Linus,

  would you please pull from

git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-udf-2.6.git for_linus

  to get the following fix:
Jan Kara (1):
      udf: Fix BUG_ON() in destroy_inode()

  with diffstat:

 fs/buffer.c    |    1 +
 fs/udf/inode.c |    1 +
 2 files changed, 2 insertions(+)

								Thanks
									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [GIT PULL] UDF tree fixes

[Linus Torvalds]

On Thu, 27 Nov 2008, Jan Kara wrote:
> 
>   would you please pull from
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-udf-2.6.git for_linus
> 
>   to get the following fix:
> Jan Kara (1):
>       udf: Fix BUG_ON() in destroy_inode()
> 
>   with diffstat:
> 
>  fs/buffer.c    |    1 +
>  fs/udf/inode.c |    1 +
>  2 files changed, 2 insertions(+)

Hmm. I really think you should use 'clear_inode()' rather than calling 
invalidate_inode_buffers() directly.

I pulled it, but I'm almost certain you did the wrong thing.

		Linus

Re: [GIT PULL] UDF tree fixes

[Jan Kara]

On Sun 30-11-08 12:58:54, Linus Torvalds wrote:
> 
> 
> On Thu, 27 Nov 2008, Jan Kara wrote:
> > 
> >   would you please pull from
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-udf-2.6.git for_linus
> > 
> >   to get the following fix:
> > Jan Kara (1):
> >       udf: Fix BUG_ON() in destroy_inode()
> > 
> >   with diffstat:
> > 
> >  fs/buffer.c    |    1 +
> >  fs/udf/inode.c |    1 +
> >  2 files changed, 2 insertions(+)
> 
> Hmm. I really think you should use 'clear_inode()' rather than calling 
> invalidate_inode_buffers() directly.
  Yes, I would like to do it as well. But as I write in the changelog,
currently there's no good callback for that (I've mailed about it at
linux-fsdevel and noone had a better idea either). So for now I've just
used this kludge to silence the Oops.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [GIT PULL] UDF tree fixes

[Linus Torvalds]

On Mon, 1 Dec 2008, Jan Kara wrote:
>
>   Yes, I would like to do it as well. But as I write in the changelog,
> currently there's no good callback for that (I've mailed about it at
> linux-fsdevel and noone had a better idea either). So for now I've just
> used this kludge to silence the Oops.

No, I meant just a simple "just call clear_inode() from 
udf_clear_inode()". But on a slightly closer look I notice that won't 
work, since it will just cause recursion (well, you could just clear the 
s_op field to avoid it, but that would be uglier than your fix).

I wonder if we should perhaps just move the invalidate_inode_buffers() 
call later in clear_inode(). That's a scary change, though. 

I just think your patch is pretty ugly. I'm sure it works, but I also 
suspect it indicates some kind of more fundamental problem. I also wonder 
why udf needs it but nobody else does (others do preallocation too)

		Linus

Re: [GIT PULL] UDF tree fixes

[Jan Kara]

On Mon 01-12-08 08:33:36, Linus Torvalds wrote:
> 
> 
> On Mon, 1 Dec 2008, Jan Kara wrote:
> >
> >   Yes, I would like to do it as well. But as I write in the changelog,
> > currently there's no good callback for that (I've mailed about it at
> > linux-fsdevel and noone had a better idea either). So for now I've just
> > used this kludge to silence the Oops.
> 
> No, I meant just a simple "just call clear_inode() from 
> udf_clear_inode()". But on a slightly closer look I notice that won't 
> work, since it will just cause recursion (well, you could just clear the 
> s_op field to avoid it, but that would be uglier than your fix).
> 
> I wonder if we should perhaps just move the invalidate_inode_buffers() 
> call later in clear_inode(). That's a scary change, though. 
> 
> I just think your patch is pretty ugly. I'm sure it works, but I also 
> suspect it indicates some kind of more fundamental problem. I also wonder 
> why udf needs it but nobody else does (others do preallocation too)
  UDF needs this because it does preallocation on directories (and I don't
know about other in-kernel fs which would do this). For regular files,
preallocation is dropped on the last close() but for directories I don't
know about such a good place so we do it in udf_clear_inode().
  There used to be put_inode() callback which could be used for this but
Christoph Hellwig killed it in April.

								Honza