lkml.org 
[lkml]   [2009]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] fs: add roundup_to_blocksize and bytes_to_blocks helper functions
    Andrew Morton pisze:
    > On Sun, 19 Apr 2009 20:42:15 +0200
    > Marcin Slusarz <marcin.slusarz@gmail.com> wrote:
    >
    >> bytes_to_blocks is open coded in more than 30 places
    >> roundup_to_blocksize is open coded in more than 15 places
    >>
    >
    > It would be nice to document these a bit. Kernel-wide helpers..

    ok

    >
    >> ---
    >> include/linux/fs.h | 12 ++++++++++++
    >> 1 files changed, 12 insertions(+), 0 deletions(-)
    >>
    >> diff --git a/include/linux/fs.h b/include/linux/fs.h
    >> index e766be0..1b4e5b6 100644
    >> --- a/include/linux/fs.h
    >> +++ b/include/linux/fs.h
    >> @@ -2451,5 +2451,17 @@ int proc_nr_files(struct ctl_table *table, int write, struct file *filp,
    >>
    >> int get_filesystem_list(char * buf);
    >>
    >> +static inline
    >> +unsigned long long roundup_to_blocksize(unsigned long long bytes, struct super_block *sb)
    >> +{
    >> + return (bytes + sb->s_blocksize - 1) & ~(sb->s_blocksize - 1);
    >
    > This is ALIGN(bytes, sb->s_blocksize). Dunno if that's any clearer though.

    thanks, I didn't know about it

    >> +}
    >> +
    >> +static inline
    >> +blkcnt_t bytes_to_blocks(unsigned long long bytes, struct super_block *sb)
    >> +{
    >> + return (bytes + sb->s_blocksize - 1) >> sb->s_blocksize_bits;
    >> +}
    >
    > I wonder whether `unsigned long long' was the best choice of type here.
    >
    > Probably `bytes' should be loff_t.

    loff_t is long long.

    So bytes_to_blocks(-6000, sb/*bs=4096, bsb=12*/) would return 4503599627370495

    > roundup_to_blocksize() could/should return loff_t too, I think.

    And roundup_to_blocksize(-6000, sb/*bs=4096, bsb=12*/) would return -4096

    Bit surprising ;)

    > roundup_to_blocksize() isn't a terribly good name, IMO. The name
    > conveys no sense of what the function returns. It _sounds_ like it
    > returns a "block size". But it doesn't - it returns a loff_t.
    > round_up_file_offset_to_block_size() is a bit long though :(

    I can't find any better name, sorry.

    Marcin



    \
     
     \ /
      Last update: 2009-04-26 19:25    [W:0.026 / U:30.960 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site