block size check in __getblk_slow

block size check in __getblk_slow

[piyush moghe]

Hi All,

I am new to linux kernel code and interested in filesystems and block I/O.

While going through the block read code ( in __bread function ) I found that
when we try to read the block ( __getblk ) we first check for the block
buffer page in page cache ( using __find_get_block function ) and if not we
do a slow path ( using __getblk_slow function which checks page in cache
again and if not found again then allocate a buffer page in cache ).

Now my problem is, as part of __getblk_slow  we compare block size with bdev
logical block size and also check that block size should not be less than
512 and not greater than PAGE_SIZE ( 4K, for PAGE_SHIFT set to 13 )
 although these all checks can be moved to upper level function i.e __getblk
and that too before calling __find_get_block ( to find block in page cache )
so as to avoid looking for block in page cache if the size is invalid.

Please let me know your comments on this.

Regards,
Piyush Moghe
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20110627/954c1be9/attachment.html 

block size check in __getblk_slow

[Jonathan Neuschäfer]

On Mon, Jun 27, 2011 at 02:23:24PM +0530, piyush moghe wrote:
> Hi All,
> 
> I am new to linux kernel code and interested in filesystems and block I/O.
> 
> While going through the block read code ( in __bread function ) I found that
> when we try to read the block ( __getblk ) we first check for the block
> buffer page in page cache ( using __find_get_block function ) and if not we
> do a slow path ( using __getblk_slow function which checks page in cache
> again and if not found again then allocate a buffer page in cache ).
> 
> Now my problem is, as part of __getblk_slow  we compare block size with bdev
> logical block size and also check that block size should not be less than
> 512 and not greater than PAGE_SIZE ( 4K, for PAGE_SHIFT set to 13 )
>  although these all checks can be moved to upper level function i.e __getblk
> and that too before calling __find_get_block ( to find block in page cache )
> so as to avoid looking for block in page cache if the size is invalid.
Sounds reasonable.

> 
> Please let me know your comments on this.

I think this is advanced enough to be discussed on the LKML, just send
an RFC (request for comments) patch and someone should comment. But
LKML can be a bit harsh if you're not used to it.

If you want to discuss it here, you might also want to send a patch
in addition to your thoughts, it's just a lot easier to get the context
and the exact thing you're trying to do from a patch than from mere
words.

HTH,
	Jonathan Neusch?fer

block size check in __getblk_slow

[piyush moghe]

Thanks Jonathan for your inputs.

Since I am new to Linux Kernel Development I am learning the process of
patch posting and development, once I will
be done with that I will post the patch for review.

Regards,
Piyush

On Thu, Jun 30, 2011 at 1:13 AM, Jonathan Neusch?fer
<j.neuschaefer@gmx.net>wrote:

> On Mon, Jun 27, 2011 at 02:23:24PM +0530, piyush moghe wrote:
> > Hi All,
> >
> > I am new to linux kernel code and interested in filesystems and block
> I/O.
> >
> > While going through the block read code ( in __bread function ) I found
> that
> > when we try to read the block ( __getblk ) we first check for the block
> > buffer page in page cache ( using __find_get_block function ) and if not
> we
> > do a slow path ( using __getblk_slow function which checks page in cache
> > again and if not found again then allocate a buffer page in cache ).
> >
> > Now my problem is, as part of __getblk_slow  we compare block size with
> bdev
> > logical block size and also check that block size should not be less than
> > 512 and not greater than PAGE_SIZE ( 4K, for PAGE_SHIFT set to 13 )
> >  although these all checks can be moved to upper level function i.e
> __getblk
> > and that too before calling __find_get_block ( to find block in page
> cache )
> > so as to avoid looking for block in page cache if the size is invalid.
> Sounds reasonable.
>
> >
> > Please let me know your comments on this.
>
> I think this is advanced enough to be discussed on the LKML, just send
> an RFC (request for comments) patch and someone should comment. But
> LKML can be a bit harsh if you're not used to it.
>
> If you want to discuss it here, you might also want to send a patch
> in addition to your thoughts, it's just a lot easier to get the context
> and the exact thing you're trying to do from a patch than from mere
> words.
>
> HTH,
>         Jonathan Neusch?fer
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20110701/9e27db9b/attachment.html