time to move fs/bio.c to block/ ?

time to move fs/bio.c to block/ ?

[Christoph Hellwig]

I recently saw patches to fs/bio.c that were sent to Al instead of Jens.
I think having bio.c in fs/ is rather confusing, so maybe it's time to
include the simple git-mv for it in the your for-next tree?

Re: time to move fs/bio.c to block/ ?

[Jens Axboe]

On 05/19/2014 08:13 AM, Christoph Hellwig wrote:
> I recently saw patches to fs/bio.c that were sent to Al instead of Jens.
> I think having bio.c in fs/ is rather confusing, so maybe it's time to
> include the simple git-mv for it in the your for-next tree?

Sure, I've been thinking that too for a while. I'll do the move.

-- 
Jens Axboe

Re: time to move fs/bio.c to block/ ?

[Al Viro]

On Mon, May 19, 2014 at 08:14:36AM -0600, Jens Axboe wrote:
> On 05/19/2014 08:13 AM, Christoph Hellwig wrote:
> > I recently saw patches to fs/bio.c that were sent to Al instead of Jens.
> > I think having bio.c in fs/ is rather confusing, so maybe it's time to
> > include the simple git-mv for it in the your for-next tree?
> 
> Sure, I've been thinking that too for a while. I'll do the move.

While you are at it, could you take bio-integrity.c with it?  _That_
has zero excuse being anywhere in fs/* - not even "filesystem code
uses quite a few functions from that sucker" as with bio.c.
FWIW, consider the move ACKed.

Re: time to move fs/bio.c to block/ ?

[Christoph Hellwig]

On Mon, May 19, 2014 at 03:25:19PM +0100, Al Viro wrote:
> While you are at it, could you take bio-integrity.c with it?  _That_
> has zero excuse being anywhere in fs/* - not even "filesystem code
> uses quite a few functions from that sucker" as with bio.c.
> FWIW, consider the move ACKed.

Various function in there aren't used at all in fact. But yes, it should
also move.

Re: time to move fs/bio.c to block/ ?

[Jens Axboe]

On 05/19/2014 08:25 AM, Al Viro wrote:
> On Mon, May 19, 2014 at 08:14:36AM -0600, Jens Axboe wrote:
>> On 05/19/2014 08:13 AM, Christoph Hellwig wrote:
>>> I recently saw patches to fs/bio.c that were sent to Al instead of Jens.
>>> I think having bio.c in fs/ is rather confusing, so maybe it's time to
>>> include the simple git-mv for it in the your for-next tree?
>>
>> Sure, I've been thinking that too for a while. I'll do the move.
> 
> While you are at it, could you take bio-integrity.c with it?  _That_
> has zero excuse being anywhere in fs/* - not even "filesystem code
> uses quite a few functions from that sucker" as with bio.c.
> FWIW, consider the move ACKed.

Yeah, I did include that in the move.

-- 
Jens Axboe

Re: time to move fs/bio.c to block/ ?

[Christoph Hellwig]

On Mon, May 19, 2014 at 08:31:21AM -0600, Jens Axboe wrote:
> > While you are at it, could you take bio-integrity.c with it?  _That_
> > has zero excuse being anywhere in fs/* - not even "filesystem code
> > uses quite a few functions from that sucker" as with bio.c.
> > FWIW, consider the move ACKed.
> 
> Yeah, I did include that in the move.

Other candidates to move to block/ might be ioprio.c and no-block.c

Re: time to move fs/bio.c to block/ ?

[Jens Axboe]

On 05/19/2014 08:34 AM, Christoph Hellwig wrote:
> On Mon, May 19, 2014 at 08:31:21AM -0600, Jens Axboe wrote:
>>> While you are at it, could you take bio-integrity.c with it?  _That_
>>> has zero excuse being anywhere in fs/* - not even "filesystem code
>>> uses quite a few functions from that sucker" as with bio.c.
>>> FWIW, consider the move ACKed.
>>
>> Yeah, I did include that in the move.
> 
> Other candidates to move to block/ might be ioprio.c and no-block.c

Yes, lets move those as well, now we're at it.

-- 
Jens Axboe

Re: time to move fs/bio.c to block/ ?

[Al Viro]

On Mon, May 19, 2014 at 07:34:16AM -0700, Christoph Hellwig wrote:
> On Mon, May 19, 2014 at 08:31:21AM -0600, Jens Axboe wrote:
> > > While you are at it, could you take bio-integrity.c with it?  _That_
> > > has zero excuse being anywhere in fs/* - not even "filesystem code
> > > uses quite a few functions from that sucker" as with bio.c.
> > > FWIW, consider the move ACKed.
> > 
> > Yeah, I did include that in the move.
> 
> Other candidates to move to block/ might be ioprio.c and no-block.c

ACK on ioprio.c (BTW, looking at block...  WTF is the story with that
pile of blk-* in there?  IOW, why blk-exec.c is better than exec.c,
etc.?)

As for fs/no-block.c...  IMO that's a bad idea - it makes sense only
if we take fs/block.c there as well, and that one wants fs/internal.h.

Why do we need that ->llseek = noop_llseek there, while we are at it?
Its ->open() always fails, so how is ->llseek() going to get looked at,
let alone called?

Re: time to move fs/bio.c to block/ ?

[Christoph Hellwig]

On Mon, May 19, 2014 at 05:39:42PM +0100, Al Viro wrote:
> ACK on ioprio.c (BTW, looking at block...  WTF is the story with that
> pile of blk-* in there?  IOW, why blk-exec.c is better than exec.c,
> etc.?)
> 
> As for fs/no-block.c...  IMO that's a bad idea - it makes sense only
> if we take fs/block.c there as well, and that one wants fs/internal.h.

Right, we still have block_dev.c which is more VFS than block.  Makes
sense to keep no-block.c then.

> Why do we need that ->llseek = noop_llseek there, while we are at it?
> Its ->open() always fails, so how is ->llseek() going to get looked at,
> let alone called?

Looks like a larger mechanical conversation of lseek instances..

Re: time to move fs/bio.c to block/ ?

[Jens Axboe]

On 05/19/2014 10:39 AM, Al Viro wrote:
> On Mon, May 19, 2014 at 07:34:16AM -0700, Christoph Hellwig wrote:
>> On Mon, May 19, 2014 at 08:31:21AM -0600, Jens Axboe wrote:
>>>> While you are at it, could you take bio-integrity.c with it?  _That_
>>>> has zero excuse being anywhere in fs/* - not even "filesystem code
>>>> uses quite a few functions from that sucker" as with bio.c.
>>>> FWIW, consider the move ACKed.
>>>
>>> Yeah, I did include that in the move.
>>
>> Other candidates to move to block/ might be ioprio.c and no-block.c
> 
> ACK on ioprio.c (BTW, looking at block...  WTF is the story with that
> pile of blk-* in there?  IOW, why blk-exec.c is better than exec.c,
> etc.?)

Intent was to separate the core code from the other code, back when it
was all split from ll_rw_blk.c. I'd still prefer it that way, as opposed
to (eg) putting it in block/core/exec.c.

-- 
Jens Axboe

Re: time to move fs/bio.c to block/ ?

[Ming Lei]

On Mon, May 19, 2014 at 10:14 PM, Jens Axboe <axboe@kernel.dk> wrote:
> On 05/19/2014 08:13 AM, Christoph Hellwig wrote:
>> I recently saw patches to fs/bio.c that were sent to Al instead of Jens.
>> I think having bio.c in fs/ is rather confusing, so maybe it's time to
>> include the simple git-mv for it in the your for-next tree?
>
> Sure, I've been thinking that too for a while. I'll do the move.

mm/bounce.c is another one.


Thanks,
-- 
Ming Lei

Re: time to move fs/bio.c to block/ ?

[Jens Axboe]

On 2014-05-19 18:28, Ming Lei wrote:
> On Mon, May 19, 2014 at 10:14 PM, Jens Axboe <axboe@kernel.dk> wrote:
>> On 05/19/2014 08:13 AM, Christoph Hellwig wrote:
>>> I recently saw patches to fs/bio.c that were sent to Al instead of Jens.
>>> I think having bio.c in fs/ is rather confusing, so maybe it's time to
>>> include the simple git-mv for it in the your for-next tree?
>>
>> Sure, I've been thinking that too for a while. I'll do the move.
>
> mm/bounce.c is another one.

True, that should be moved as well.

-- 
Jens Axboe

Re: time to move fs/bio.c to block/ ?

[Jianyu Zhan]

On Mon, May 19, 2014 at 10:13 PM, Christoph Hellwig <hch@infradead.org> wrote:
> I recently saw patches to fs/bio.c that were sent to Al instead of Jens.
> I think having bio.c in fs/ is rather confusing, so maybe it's time to
> include the simple git-mv for it in the your for-next tree?

Hi, Christoph, Jens,

BTW, just out of curiosity,  the VFS infrastructure code is just scatterd
around the fs directory, which is quite suprised to a new comer that why
there is "no" vfs stuff in fs directory.  Does it make sense to also collect
them into a dedicated sub-dir, maybe vfs.  IMHO, this could make code
skeleton more clear and could avoid such mis-sending patches in a long
term maintainability view.

Thanks,
Jianyu Zhan

Re: time to move fs/bio.c to block/ ?

[Christoph Hellwig]

On Mon, May 19, 2014 at 10:28:16PM +0800, Jianyu Zhan wrote:
> Hi, Christoph, Jens,
> 
> BTW, just out of curiosity,  the VFS infrastructure code is just scatterd
> around the fs directory, which is quite suprised to a new comer that why
> there is "no" vfs stuff in fs directory.  Does it make sense to also collect
> them into a dedicated sub-dir, maybe vfs.  IMHO, this could make code
> skeleton more clear and could avoid such mis-sending patches in a long
> term maintainability view.

fs/*.[ch] shouldn't be much that isn't VFS in the broader sense
(including library functions). Besides the block files the only the only
things that might make sense to move out are binfmt*.c, signalfd.c
and timerfd.c (to kernel/ ?).

> 
> Thanks,
> Jianyu Zhan
---end quoted text---