Re: [linux-next:master] BUILD REGRESSION 6dc544b66971c7f9909ff038b62149105272d26a

[David Sterba]

On Wed, May 29, 2024 at 02:19:47AM +0800, kernel test robot wrote:
> tree/branch: 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> branch HEAD: 6dc544b66971c7f9909ff038b62149105272d26a  Add linux-next 
> specific files for 20240528
> 
> Error/Warning reports:
> 
> https://lore.kernel.org/oe-kbuild-all/202405282036.maedo54q-...@intel.com
> https://lore.kernel.org/oe-kbuild-all/202405282148.jaf0flhu-...@intel.com
> https://lore.kernel.org/oe-kbuild-all/202405282308.uezt6hqc-...@intel.com
> 
> Error/Warning: (recently discovered and may have been fixed)
> 
> drivers/dma-buf/udmabuf.c:45:(.text+0x140): undefined reference to 
> `vmf_insert_pfn'
> fs/btrfs/fiemap.c:822:26: warning: 'last_extent_end' may be used 
> uninitialized [-Wmaybe-uninitialized]

The report says it's gcc 13.2, that one I use (and expect others as well
as it's a recent one) and we also have -Wmaybe-uninitialized enabled in
fs/btrfs/ to catch such warnings. Yet this is reported on mips64, is
there something special about that compiler+architecture?

The warning is IMO a false positive, the maybe-uninitialized variable is
passed as pointer but initialized on success and never used on failure.
We can safely silence the warning by initializing the variable to 0 but
this may be pointing to a problem with mips64+gcc namely because other
compiler+host combinations do not warn abou that.

Re: [PATCH next 09/11] tree-wide: minmax: Replace all the uses of max() for array sizes with max_const()

[David Sterba]

On Sun, Jan 28, 2024 at 07:34:23PM +, David Laight wrote:
> These are the only uses of max() that require a constant value
> from constant parameters.
> There don't seem to be any similar uses of min().
> 
> Replacing the max() by max_const() lets min()/max() be simplified
> speeding up compilation.
> 
> max_const() will convert enums to int (or unsigned int) so that the
> casts added by max_t() are no longer needed.
> 
> Signed-off-by: David Laight 
> ---

For

>  fs/btrfs/tree-checker.c        | 2 +-

Acked-by: David Sterba 

Re: [PATCH 04/12] btrfs: send: Proactively round up to kmalloc bucket size

[David Sterba]

On Wed, Sep 21, 2022 at 08:10:05PM -0700, Kees Cook wrote:
> Instead of discovering the kmalloc bucket size _after_ allocation, round
> up proactively so the allocation is explicitly made for the full size,
> allowing the compiler to correctly reason about the resulting size of
> the buffer through the existing __alloc_size() hint.
> 
> Cc: linux-bt...@vger.kernel.org
> Signed-off-by: Kees Cook 

Acked-by: David Sterba 

Re: [PATCH] [v2] Kbuild: move to -std=gnu11

[David Sterba]

On Mon, Feb 28, 2022 at 02:01:06PM +0100, Arnd Bergmann wrote:
> On Mon, Feb 28, 2022 at 1:36 PM Jani Nikula  
> wrote:
> > >
> > > One minor issue that remains is an added gcc warning for shifts of
> > > negative integers when building with -Werror, which happens with the
> > > 'make W=1' option, as well as for three drivers in the kernel that always
> > > enable -Werror, but it was only observed with the i915 driver so far.
> > > To be on the safe side, add -Wno-shift-negative-value to any -Wextra
> > > in a Makefile.
> >
> > Do you mean always enable -Wall and/or -Wextra instead of -Werror?
> >
> > We do use -Werror for our CI and development too, but it's hidden behind
> > a config option that depends on COMPILE_TEST=n to avoid any problems
> > with allmodconfig/allyesconfig.
> 
> What I meant here is that I'm adding -Wno-shift-negative-value to all
> four places in the kernel that already use -Wextra.
> 
> > For the future, makes me wonder if we couldn't have a way for drivers to
> > locally enable -Wall/-Wextra in a way that incorporates the exceptions
> > from kbuild instead of having to duplicate them.
> 
> I have an older patch series that does this, but it also does a few other
> things that I couldn't quite get right in the end with all combinations of
> compiler versions and warning levels, so I did not submit that.
> 
> What this allows is to have per-directory statements like
> 
> KBUILD_WARN1=1

We've added the individual warnings to the per-directory flags so it's a
bit more flexible than just enabling W=1. The idea is to add possibly
stricter warnings once we make sure it builds fine and does not
produce false reports. Extending W=1 in the same way would affect
everybody.

Re: [PATCH] [v2] Kbuild: move to -std=gnu11

[David Sterba]

On Mon, Feb 28, 2022 at 11:27:43AM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> 
> Link: 
> https://lore.kernel.org/lkml/CAHk-=wiych7xehcmifj-ygxuy2jaj7pnkdkpcovt8fybvfw...@mail.gmail.com/
> Link: https://github.com/ClangBuiltLinux/linux/issues/1603
> Suggested-by: Linus Torvalds 
> Cc: Masahiro Yamada 
> Cc: linux-kbu...@vger.kernel.org
> Cc: l...@lists.linux.dev
> Signed-off-by: Arnd Bergmann 
> ---
> [v2]
>  - added -std=gnu11 back, rather than just relying on the default
>  - minor changes to changelog text
> ---
>  Documentation/process/programming-language.rst  | 4 ++--
>  .../translations/it_IT/process/programming-language.rst | 4 ++--
>  .../translations/zh_CN/process/programming-language.rst | 4 ++--
>  .../translations/zh_TW/process/programming-language.rst | 4 ++--
>  Makefile| 6 +++---
>  arch/arm64/kernel/vdso32/Makefile   | 2 +-
>  drivers/gpu/drm/i915/Makefile   | 1 +
>  drivers/staging/greybus/tools/Makefile  | 3 ++-

For

>  fs/btrfs/Makefile   | 1 +

Acked-by: David Sterba 

Re: [PATCH v2 49/63] btrfs: Use memset_startat() to clear end of struct

[David Sterba]

On Tue, Aug 17, 2021 at 11:05:19PM -0700, Kees Cook wrote:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memset(), avoid intentionally writing across
> neighboring fields.
> 
> Use memset_startat() so memset() doesn't get confused about writing
> beyond the destination member that is intended to be the starting point
> of zeroing through the end of the struct.
> 
> Cc: Chris Mason 
> Cc: Josef Bacik 
> Cc: David Sterba 
> Cc: linux-bt...@vger.kernel.org
> Signed-off-by: Kees Cook 

Acked-by: David Sterba 

Re: [PATCH 47/64] btrfs: Use memset_after() to clear end of struct

[David Sterba]

On Sat, Jul 31, 2021 at 08:25:51AM -0700, Kees Cook wrote:
> On Thu, Jul 29, 2021 at 12:33:37PM +0200, David Sterba wrote:
> > On Wed, Jul 28, 2021 at 02:56:31PM -0700, Kees Cook wrote:
> > > On Wed, Jul 28, 2021 at 11:42:15AM +0200, David Sterba wrote:
> > > > On Tue, Jul 27, 2021 at 01:58:38PM -0700, Kees Cook wrote:
> > > > >   }
> > > > >   if (need_reset) {
> > > > > - memset(>generation_v2, 0,
> > > > > - sizeof(*item) - offsetof(struct btrfs_root_item,
> > > > > - generation_v2));
> > > > > -
> > > > 
> > > > Please add
> > > > /* Clear all members from generation_v2 onwards */
> > > > 
> > > > > + memset_after(item, 0, level);
> > > 
> > > Perhaps there should be another helper memset_starting()? That would
> > > make these cases a bit more self-documenting.
> > 
> > That would be better, yes.
> > 
> > > + memset_starting(item, 0, generation_v2);
> > 
> > memset_from?
> 
> For v2, I bikeshed this to "memset_startat" since "from" is semantically
> close to "source" which I thought might be confusing. (I, too, did not
> like "starting".) :)

memset_startat works for me, thanks.

Re: [PATCH 01/64] media: omap3isp: Extract struct group for memcpy() region

[David Sterba]

On Thu, Jul 29, 2021 at 11:00:48PM -0700, Kees Cook wrote:
> On Thu, Jul 29, 2021 at 11:20:39AM +0300, Dan Carpenter wrote:
> > On Thu, Jul 29, 2021 at 07:56:27AM +0200, Greg Kroah-Hartman wrote:
> > > On Wed, Jul 28, 2021 at 11:37:30PM +0200, David Sterba wrote:
> > > > On Wed, Jul 28, 2021 at 02:37:20PM -0700, Bart Van Assche wrote:
> > > > > On 7/28/21 2:14 AM, Dan Carpenter wrote:
> > > > > > On Wed, Jul 28, 2021 at 10:59:22AM +0200, David Sterba wrote:
> > > > > >>>   drivers/media/platform/omap3isp/ispstat.c |  5 +--
> > > > > >>>   include/uapi/linux/omap3isp.h | 44 
> > > > > >>> +--
> > > > > >>>   2 files changed, 36 insertions(+), 13 deletions(-)
> > > > > >>>
> > > > > >>> diff --git a/drivers/media/platform/omap3isp/ispstat.c 
> > > > > >>> b/drivers/media/platform/omap3isp/ispstat.c
> > > > > >>> index 5b9b57f4d9bf..ea8222fed38e 100644
> > > > > >>> --- a/drivers/media/platform/omap3isp/ispstat.c
> > > > > >>> +++ b/drivers/media/platform/omap3isp/ispstat.c
> > > > > >>> @@ -512,7 +512,7 @@ int omap3isp_stat_request_statistics(struct 
> > > > > >>> ispstat *stat,
> > > > > >>>   int omap3isp_stat_request_statistics_time32(struct ispstat 
> > > > > >>> *stat,
> > > > > >>>   struct 
> > > > > >>> omap3isp_stat_data_time32 *data)
> > > > > >>>   {
> > > > > >>> - struct omap3isp_stat_data data64;
> > > > > >>> + struct omap3isp_stat_data data64 = { };
> > > > > >>
> > > > > >> Should this be { 0 } ?
> > > > > >>
> > > > > >> We've seen patches trying to switch from { 0 } to {  } but the 
> > > > > >> answer
> > > > > >> was that { 0 } is supposed to be used,
> > > > > >> http://www.ex-parrot.com/~chris/random/initialise.html 
> > > > > >>
> > > > > >> (from 
> > > > > >> https://lore.kernel.org/lkml/fbddb15a-6e46-3f21-23ba-b18f66e34...@suse.com/
> > > > > >>  )
> > > > > > 
> > > > > > In the kernel we don't care about portability so much.  Use the = { 
> > > > > > }
> > > > > > GCC extension.  If the first member of the struct is a pointer then
> > > > > > Sparse will complain about = { 0 }.
> > > > > 
> > > > > +1 for { }.
> > > > 
> > > > Oh, I thought the tendency is is to use { 0 } because that can also
> > > > intialize the compound members, by a "scalar 0" as it appears in the
> > > > code.
> > > > 
> > > 
> > > Holes in the structure might not be initialized to anything if you do
> > > either one of these as well.
> > > 
> > > Or did we finally prove that is not the case?  I can not remember
> > > anymore...
> > 
> > Yep.  The C11 spec says that struct holes are initialized.
> > 
> > https://lore.kernel.org/netdev/20200731140452.ge24...@ziepe.ca/
> 
> This is, unfortunately, misleading. The frustrating key word is
> "partial" in "updated in C11 to require zero'ing padding when doing
> partial initialization of aggregates". If one initializes _all_ the
> struct members ... the padding doesn't get initialized. :( (And until
> recently, _trailing_ padding wasn't getting initialized even when other
> paddings were.)
> 
> I've tried to collect all the different ways the compiler might initialize
> a variable in this test:
> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/tree/lib/test_stackinit.c?h=for-next/kspp
> 
> FWIW, there's no difference between -std=gnu99 and -std=c11, and the
> test shows that padding is _not_ universally initialized (unless your
> compiler supports -ftrivial-auto-var-init=zero, which Clang does, and
> GCC will shortly[1]). Running this with GCC 10.3.0, I see this...
> 
> As expected, having no initializer leaves padding (as well as members)
> uninitialized:
> 
> stackinit: small_hole_none FAIL (uninit bytes: 24)
> stackinit: big_hole_none FAIL (uninit bytes: 128)
> stackinit: trailing_hole_none FAIL (uninit bytes: 32)
> 
> Here, "zero" means  "= { };" and they get pa

Re: [PATCH 02/64] mac80211: Use flex-array for radiotap header bitmap

[David Sterba]

On Wed, Jul 28, 2021 at 02:54:52PM -0700, Kees Cook wrote:
> On Wed, Jul 28, 2021 at 11:23:23AM +0200, David Sterba wrote:
> > On Wed, Jul 28, 2021 at 10:35:56AM +0300, Dan Carpenter wrote:
> > > @@ -372,7 +372,7 @@ ieee80211_add_rx_radiotap_header(struct 
> > > ieee80211_local *local,
> > >   ieee80211_calculate_rx_timestamp(local, status,
> > >mpdulen, 0),
> > >   pos);
> > > - rthdr->it_present |= cpu_to_le32(1 << IEEE80211_RADIOTAP_TSFT);
> > > + rthdr->data.it_present |= cpu_to_le32(1 << 
> > > IEEE80211_RADIOTAP_TSFT);
> > 
> > A drive-by comment, not related to the patchset, but rather the
> > ieee80211 driver itself.
> > 
> > Shift expressions with (1 << NUMBER) can be subtly broken once the
> > NUMBER is 31 and the value gets silently cast to a 64bit type. It will
> > become 0xf8000.
> > 
> > I've checked the IEEE80211_RADIOTAP_* defintions if this is even remotely
> > possible and yes, IEEE80211_RADIOTAP_EXT == 31. Fortunatelly it seems to
> > be used with used with a 32bit types (eg. _bitmap_shifter) so there are
> > no surprises.
> > 
> > The recommended practice is to always use unsigned types for shifts, so
> > "1U << ..." at least.
> 
> Ah, good catch! I think just using BIT() is the right replacement here,
> yes? I suppose that should be a separate patch.

I found definition of BIT in vdso/bits.h, that does not sound like a
standard header, besides that it shifts 1UL, that may not be necessary
everywhere. IIRC there were objections against using the macro at all.

Looking for all the definitions, there are a few that are wrong in the
sense they're using the singed type, eg.

https://elixir.bootlin.com/linux/v5.14-rc3/source/arch/arm/mach-davinci/sleep.S#L7

#define BIT(nr) (1 << (nr))
...
#define DEEPSLEEP_SLEEPENABLE_BIT   BIT(31)

but that's an assembly file so the C integer promotions don't apply.

https://elixir.bootlin.com/linux/v5.14-rc3/source/drivers/staging/rtl8723bs/include/osdep_service.h#L18
https://elixir.bootlin.com/linux/v5.14-rc3/source/drivers/staging/rtl8723bs/include/wifi.h#L15
https://elixir.bootlin.com/linux/v5.14-rc3/source/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c#L15

#define BIT(x)  (1 << (x))

Auditing and cleaning that up is for another series, yeah, I'm just
pointing it here if somebody feels like doing the work. It's IMO low
hanging fruit but can reveal real bugs.

Re: [PATCH 47/64] btrfs: Use memset_after() to clear end of struct

[David Sterba]

On Wed, Jul 28, 2021 at 02:56:31PM -0700, Kees Cook wrote:
> On Wed, Jul 28, 2021 at 11:42:15AM +0200, David Sterba wrote:
> > On Tue, Jul 27, 2021 at 01:58:38PM -0700, Kees Cook wrote:
> > > In preparation for FORTIFY_SOURCE performing compile-time and run-time
> > > field bounds checking for memset(), avoid intentionally writing across
> > > neighboring fields.
> > > 
> > > Use memset_after() so memset() doesn't get confused about writing
> > > beyond the destination member that is intended to be the starting point
> > > of zeroing through the end of the struct.
> > > 
> > > Signed-off-by: Kees Cook 
> > > ---
> > >  fs/btrfs/root-tree.c | 5 +
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > 
> > > diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> > > index 702dc5441f03..ec9e78f65fca 100644
> > > --- a/fs/btrfs/root-tree.c
> > > +++ b/fs/btrfs/root-tree.c
> > > @@ -39,10 +39,7 @@ static void btrfs_read_root_item(struct extent_buffer 
> > > *eb, int slot,
> > >   need_reset = 1;
> > >   }
> > >   if (need_reset) {
> > > - memset(>generation_v2, 0,
> > > - sizeof(*item) - offsetof(struct btrfs_root_item,
> > > - generation_v2));
> > > -
> > 
> > Please add
> > /* Clear all members from generation_v2 onwards */
> > 
> > > + memset_after(item, 0, level);
> 
> Perhaps there should be another helper memset_starting()? That would
> make these cases a bit more self-documenting.

That would be better, yes.

> + memset_starting(item, 0, generation_v2);

memset_from?

Re: [PATCH 01/64] media: omap3isp: Extract struct group for memcpy() region

[David Sterba]

On Wed, Jul 28, 2021 at 02:37:20PM -0700, Bart Van Assche wrote:
> On 7/28/21 2:14 AM, Dan Carpenter wrote:
> > On Wed, Jul 28, 2021 at 10:59:22AM +0200, David Sterba wrote:
> >>>   drivers/media/platform/omap3isp/ispstat.c |  5 +--
> >>>   include/uapi/linux/omap3isp.h | 44 +--
> >>>   2 files changed, 36 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/drivers/media/platform/omap3isp/ispstat.c 
> >>> b/drivers/media/platform/omap3isp/ispstat.c
> >>> index 5b9b57f4d9bf..ea8222fed38e 100644
> >>> --- a/drivers/media/platform/omap3isp/ispstat.c
> >>> +++ b/drivers/media/platform/omap3isp/ispstat.c
> >>> @@ -512,7 +512,7 @@ int omap3isp_stat_request_statistics(struct ispstat 
> >>> *stat,
> >>>   int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
> >>>   struct 
> >>> omap3isp_stat_data_time32 *data)
> >>>   {
> >>> - struct omap3isp_stat_data data64;
> >>> + struct omap3isp_stat_data data64 = { };
> >>
> >> Should this be { 0 } ?
> >>
> >> We've seen patches trying to switch from { 0 } to {  } but the answer
> >> was that { 0 } is supposed to be used,
> >> http://www.ex-parrot.com/~chris/random/initialise.html
> >>
> >> (from 
> >> https://lore.kernel.org/lkml/fbddb15a-6e46-3f21-23ba-b18f66e34...@suse.com/)
> > 
> > In the kernel we don't care about portability so much.  Use the = { }
> > GCC extension.  If the first member of the struct is a pointer then
> > Sparse will complain about = { 0 }.
> 
> +1 for { }.

Oh, I thought the tendency is is to use { 0 } because that can also
intialize the compound members, by a "scalar 0" as it appears in the
code.

Re: [PATCH 01/64] media: omap3isp: Extract struct group for memcpy() region

[David Sterba]

On Tue, Jul 27, 2021 at 01:57:52PM -0700, Kees Cook wrote:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memcpy(), memmove(), and memset(), avoid
> intentionally writing across neighboring fields.  Wrap the target region
> in a common named structure. This additionally fixes a theoretical
> misalignment of the copy (since the size of "buf" changes between 64-bit
> and 32-bit, but this is likely never built for 64-bit).
> 
> FWIW, I think this code is totally broken on 64-bit (which appears to
> not be a "real" build configuration): it would either always fail (with
> an uninitialized data->buf_size) or would cause corruption in userspace
> due to the copy_to_user() in the call path against an uninitialized
> data->buf value:
> 
> omap3isp_stat_request_statistics_time32(...)
> struct omap3isp_stat_data data64;
> ...
> omap3isp_stat_request_statistics(stat, );
> 
> int omap3isp_stat_request_statistics(struct ispstat *stat,
>  struct omap3isp_stat_data *data)
> ...
> buf = isp_stat_buf_get(stat, data);
> 
> static struct ispstat_buffer *isp_stat_buf_get(struct ispstat *stat,
>struct omap3isp_stat_data 
> *data)
> ...
> if (buf->buf_size > data->buf_size) {
> ...
> return ERR_PTR(-EINVAL);
> }
> ...
> rval = copy_to_user(data->buf,
> buf->virt_addr,
> buf->buf_size);
> 
> Regardless, additionally initialize data64 to be zero-filled to avoid
> undefined behavior.
> 
> Fixes: 378e3f81cb56 ("media: omap3isp: support 64-bit version of 
> omap3isp_stat_data")
> Signed-off-by: Kees Cook 
> ---
>  drivers/media/platform/omap3isp/ispstat.c |  5 +--
>  include/uapi/linux/omap3isp.h | 44 +--
>  2 files changed, 36 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/ispstat.c 
> b/drivers/media/platform/omap3isp/ispstat.c
> index 5b9b57f4d9bf..ea8222fed38e 100644
> --- a/drivers/media/platform/omap3isp/ispstat.c
> +++ b/drivers/media/platform/omap3isp/ispstat.c
> @@ -512,7 +512,7 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
>  int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
>   struct omap3isp_stat_data_time32 *data)
>  {
> - struct omap3isp_stat_data data64;
> + struct omap3isp_stat_data data64 = { };

Should this be { 0 } ?

We've seen patches trying to switch from { 0 } to {  } but the answer
was that { 0 } is supposed to be used,
http://www.ex-parrot.com/~chris/random/initialise.html

(from 
https://lore.kernel.org/lkml/fbddb15a-6e46-3f21-23ba-b18f66e34...@suse.com/)

Re: [PATCH 47/64] btrfs: Use memset_after() to clear end of struct

[David Sterba]

On Tue, Jul 27, 2021 at 01:58:38PM -0700, Kees Cook wrote:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memset(), avoid intentionally writing across
> neighboring fields.
> 
> Use memset_after() so memset() doesn't get confused about writing
> beyond the destination member that is intended to be the starting point
> of zeroing through the end of the struct.
> 
> Signed-off-by: Kees Cook 
> ---
>  fs/btrfs/root-tree.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> index 702dc5441f03..ec9e78f65fca 100644
> --- a/fs/btrfs/root-tree.c
> +++ b/fs/btrfs/root-tree.c
> @@ -39,10 +39,7 @@ static void btrfs_read_root_item(struct extent_buffer *eb, 
> int slot,
>   need_reset = 1;
>   }
>   if (need_reset) {
> - memset(>generation_v2, 0,
> - sizeof(*item) - offsetof(struct btrfs_root_item,
> - generation_v2));
> -

Please add
/* Clear all members from generation_v2 onwards */

> + memset_after(item, 0, level);
>   generate_random_guid(item->uuid);

Acked-by: David Sterba 

Re: [PATCH 02/64] mac80211: Use flex-array for radiotap header bitmap

[David Sterba]

On Wed, Jul 28, 2021 at 10:35:56AM +0300, Dan Carpenter wrote:
> @@ -372,7 +372,7 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local 
> *local,
>   ieee80211_calculate_rx_timestamp(local, status,
>mpdulen, 0),
>   pos);
> - rthdr->it_present |= cpu_to_le32(1 << IEEE80211_RADIOTAP_TSFT);
> + rthdr->data.it_present |= cpu_to_le32(1 << 
> IEEE80211_RADIOTAP_TSFT);

A drive-by comment, not related to the patchset, but rather the
ieee80211 driver itself.

Shift expressions with (1 << NUMBER) can be subtly broken once the
NUMBER is 31 and the value gets silently cast to a 64bit type. It will
become 0xf8000.

I've checked the IEEE80211_RADIOTAP_* defintions if this is even remotely
possible and yes, IEEE80211_RADIOTAP_EXT == 31. Fortunatelly it seems to
be used with used with a 32bit types (eg. _bitmap_shifter) so there are
no surprises.

The recommended practice is to always use unsigned types for shifts, so
"1U << ..." at least.

Re: [patch V3 03/37] fs: Remove asm/kmap_types.h includes

[David Sterba]

On Tue, Nov 03, 2020 at 10:27:15AM +0100, Thomas Gleixner wrote:
> Historical leftovers from the time where kmap() had fixed slots.
> 
> Signed-off-by: Thomas Gleixner 
> Cc: Alexander Viro 
> Cc: Benjamin LaHaise 
> Cc: linux-fsde...@vger.kernel.org
> Cc: linux-...@kvack.org
> Cc: Chris Mason 
> Cc: Josef Bacik 
> Cc: David Sterba 

Acked-by: David Sterba 

For the btrfs bits

>  fs/btrfs/ctree.h |1 -

> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -17,7 +17,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/35] docs: filesystems: fix renamed references

[David Sterba]

On Wed, Apr 08, 2020 at 05:45:57PM +0200, Mauro Carvalho Chehab wrote:
> Some filesystem references got broken by a previous patch
> series I submitted. Address those.
> 
> Signed-off-by: Mauro Carvalho Chehab 

For

>  fs/affs/Kconfig | 2 +-

Acked-by: David Sterba 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg

[David Sterba]

On Wed, Sep 12, 2018 at 05:08:52PM +0200, Arnd Bergmann wrote:
> The .ioctl and .compat_ioctl file operations have the same prototype so
> they can both point to the same function, which works great almost all
> the time when all the commands are compatible.
> 
> One exception is the s390 architecture, where a compat pointer is only
> 31 bit wide, and converting it into a 64-bit pointer requires calling
> compat_ptr(). Most drivers here will ever run in s390, but since we now
> have a generic helper for it, it's easy enough to use it consistently.
> 
> I double-checked all these drivers to ensure that all ioctl arguments
> are used as pointers or are ignored, but are not interpreted as integer
> values.
> 
> Signed-off-by: Arnd Bergmann 
> ---

>  fs/btrfs/super.c        | 2 +-

Acked-by: David Sterba 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/14] btrfs: separate errno from VM_FAULT_* values

[David Sterba]

On Wed, May 16, 2018 at 07:43:40AM +0200, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <h...@lst.de>

Reviewed-by: David Sterba <dste...@suse.com>

I can add it to the btrfs queue now, unless you need the patch for the
rest of the series.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 10/10] fs:btrfs: return -ENOMEM on allocation failure.

[David Sterba]

On Wed, Sep 13, 2017 at 01:02:19PM +0530, Allen Pais wrote:
> Signed-off-by: Allen Pais 
> ---
>  fs/btrfs/check-integrity.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
> index 7d5a9b5..efa4c23 100644
> --- a/fs/btrfs/check-integrity.c
> +++ b/fs/btrfs/check-integrity.c
> @@ -2913,7 +2913,7 @@ int btrfsic_mount(struct btrfs_fs_info *fs_info,
>   state = kvzalloc(sizeof(*state), GFP_KERNEL);
>   if (!state) {
>   pr_info("btrfs check-integrity: allocation failed!\n");
> - return -1;
> + return -ENOMEM;

Makes sense, also please fix the -1 a few lines below that also result
from failed memory allocation, indirectly from btrfsic_dev_state_alloc().

>   }
>  
>   if (!btrfsic_is_initialized) {
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

merging printk and WARN

[David Sterba]

On Sun, Nov 04, 2012 at 09:25:53PM +0100, Julia Lawall wrote:
> It looks like these patches were not a good idea, because in each case the
> printk provides an error level, and WARN then provides another one.

I think this is not a problem within btrfs at the place where this has
changed.

david

Re: merging printk and WARN

[David Sterba]

On Sun, Nov 04, 2012 at 09:25:53PM +0100, Julia Lawall wrote:
 It looks like these patches were not a good idea, because in each case the
 printk provides an error level, and WARN then provides another one.

I think this is not a problem within btrfs at the place where this has
changed.

david
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel