Re: [PATCH] ext4: fixup 64-bit divides in linux-3.0 stable backport of upstream fix

Re: [PATCH] ext4: fixup 64-bit divides in linux-3.0 stable backport of upstream fix

[Todd Poynor]

On Fri, Apr 5, 2013 at 4:01 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Apr 05, 2013 at 03:54:23PM -0700, Todd Poynor wrote:
> > Replace C division operators with div64_u64 for divides introduced in:
> > commit 503f4bdcc078e7abee273a85ce322de81b18a224
> > ext4: use atomic64_t for the per-flexbg free_clusters count
> >
> > Specific to the linux-3.0 backport of the upstream patch.
>
> Why is this specific?  Why is this working differently in 3.0 from 3.4
> and newer?

Looks like 3.1 doesn't have this patch.  The 3.2-stable backport of
this patch, for example, does not make some additional changes made in
the 3.0-stable backport, in addition to the upstream patch being
backported.  These additional changes include converting
blocks_per_flex and flexbg_free_blocks from int to ext4_fsblk_t (a
64-bit type), probably as a part of these additional changes mentioned
in the commit message:

   [Backported for 3.0-stable. Renamed free_clusters back to free_blocks;
    fixed a few more atomic_read's of free_blocks left in 3.0.]

Perhaps the better fix is to revert those additional changes and let
the ext4 folks figure out what to do about the remaining atomic_reads.

> And care to cc: the ext4 developers/maintainers when sending ext4
> patches?

cc'ed now, apologies.

>
> thanks,
>
> greg k-h

Re: [PATCH] ext4: fixup 64-bit divides in linux-3.0 stable backport of upstream fix

[Christoph Biedl]

Todd Poynor wrote...

> On Fri, Apr 5, 2013 at 4:01 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Apr 05, 2013 at 03:54:23PM -0700, Todd Poynor wrote:
> > > Replace C division operators with div64_u64 for divides introduced in:
> > > commit 503f4bdcc078e7abee273a85ce322de81b18a224
> > > ext4: use atomic64_t for the per-flexbg free_clusters count
> > >
> > > Specific to the linux-3.0 backport of the upstream patch.
> >
> > Why is this specific?  Why is this working differently in 3.0 from 3.4
> > and newer?
> 
> Looks like 3.1 doesn't have this patch.

Just comparing the "ext4: use atomic64_t for the per-flexbg
free_clusters count" patches for 3.0 and 3.4, your fix and git: The
build errors in 3.0.72 your patch fixes originate in find_group_flex
which was removed in v3.1-rc3-44-g4113c4c, hence only 3.0 was
affected.

    Christoph

Re: [PATCH] ext4: fixup 64-bit divides in linux-3.0 stable backport of upstream fix

[Theodore Ts'o]

On Fri, Apr 05, 2013 at 04:18:15PM -0700, Todd Poynor wrote:
> Perhaps the better fix is to revert those additional changes and let
> the ext4 folks figure out what to do about the remaining atomic_reads.
> 
> > And care to cc: the ext4 developers/maintainers when sending ext4
> > patches?

Can you send me the original patch?  I'd like to fix the problem
upstream first....  I'm curious why I didn't notice this, since I run
tests (and thus build) 32-bit x86 kernels, and Eric Whitney is doing
test builds and runs regression tests an ARM Pandaboard.  Can you give
me details about your compilation environment and how this causing
problems for you?

Thanks,

						- Ted

Re: [PATCH] ext4: fixup 64-bit divides in linux-3.0 stable backport of upstream fix

[Theodore Ts'o]

Greg,

The reason why Todd's patch is needed in 3.0 is because the code in
question was nuked as of 3.2 by commit 4113c4caa4f355:

commit 4113c4caa4f355b8ff8b7ff0510c29c9d00d30b3
Author: Lukas Czerner <lczerner@redhat.com>
Date:   Sat Oct 8 14:34:47 2011 -0400

    ext4: remove deprecated oldalloc
    
    For a long time now orlov is the default block allocator in the
    ext4. It performs better than the old one and no one seems to claim
    otherwise so we can safely drop it and make oldalloc and orlov mount
    option deprecated.
    
    This is a part of the effort to reduce number of ext4 options hence the
    test matrix.
    
    Signed-off-by: Lukas Czerner <lczerner@redhat.com>
    Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

So the 64-bit divides aren't in the upstream kernel because the code
in question is long gone in the mainline ext4 code.  It looks like Cai
(who did the 3.0 backport) never tested the patch on a 32-bit build.

Thanks Todd, for providing the patch.  You can add my:

Reviewed-by: "Theodore Ts'o" <tytso@mit.edu>

to it.  Greg, could you apply it to the stable tree?   Thanks!!

				- Ted

Re: [PATCH] ext4: fixup 64-bit divides in linux-3.0 stable backport of upstream fix

[Greg KH]

On Fri, Apr 05, 2013 at 07:50:18PM -0400, Theodore Ts'o wrote:
> Greg,
> 
> The reason why Todd's patch is needed in 3.0 is because the code in
> question was nuked as of 3.2 by commit 4113c4caa4f355:
> 
> commit 4113c4caa4f355b8ff8b7ff0510c29c9d00d30b3
> Author: Lukas Czerner <lczerner@redhat.com>
> Date:   Sat Oct 8 14:34:47 2011 -0400
> 
>     ext4: remove deprecated oldalloc
>     
>     For a long time now orlov is the default block allocator in the
>     ext4. It performs better than the old one and no one seems to claim
>     otherwise so we can safely drop it and make oldalloc and orlov mount
>     option deprecated.
>     
>     This is a part of the effort to reduce number of ext4 options hence the
>     test matrix.
>     
>     Signed-off-by: Lukas Czerner <lczerner@redhat.com>
>     Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> 
> So the 64-bit divides aren't in the upstream kernel because the code
> in question is long gone in the mainline ext4 code.  It looks like Cai
> (who did the 3.0 backport) never tested the patch on a 32-bit build.
> 
> Thanks Todd, for providing the patch.  You can add my:
> 
> Reviewed-by: "Theodore Ts'o" <tytso@mit.edu>
> 
> to it.  Greg, could you apply it to the stable tree?   Thanks!!

Thanks for the review, will apply it for the next release, thanks.

greg k-h