Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()

[Darrick J. Wong]

On Thu, May 16, 2024 at 01:34:54PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" 
> 
> [
>This is a treewide change. I will likely re-create this patch again in
>the second week of the merge window of v6.10 and submit it then. Hoping
>to keep the conflicts that it will cause to a minimum.
> ]
> 
> With the rework of how the __string() handles dynamic strings where it
> saves off the source string in field in the helper structure[1], the
> assignment of that value to the trace event field is stored in the helper
> value and does not need to be passed in again.
> 
> This means that with:
> 
>   __string(field, mystring)
> 
> Which use to be assigned with __assign_str(field, mystring), no longer
> needs the second parameter and it is unused. With this, __assign_str()
> will now only get a single parameter.
> 
> There's over 700 users of __assign_str() and because coccinelle does not
> handle the TRACE_EVENT() macro I ended up using the following sed script:
> 
>   git grep -l __assign_str | while read a ; do
>   sed -e 's/\(__assign_str([^,]*[^ ,]\) *,[^;]*/\1)/' $a > /tmp/test-file;
>   mv /tmp/test-file $a;
>   done
> 
> I then searched for __assign_str() that did not end with ';' as those
> were multi line assignments that the sed script above would fail to catch.
> 
> Note, the same updates will need to be done for:
> 
>   __assign_str_len()
>   __assign_rel_str()
>   __assign_rel_str_len()
> 
> I tested this with both an allmodconfig and an allyesconfig (build only for 
> both).
> 
> [1] 
> https://lore.kernel.org/linux-trace-kernel/2024011442.634192...@goodmis.org/
> 
> Cc: Masami Hiramatsu 
> Cc: Mathieu Desnoyers 
> Cc: Linus Torvalds 
> Cc: Julia Lawall 
> Signed-off-by: Steven Rostedt (Google) 

/me finds this pretty magical, but such is the way of macros.
Thanks for being much smarter about them than me. :)

Acked-by: Darrick J. Wong# xfs

--D

Re: [RFC PATCH 2/3] fs: remove duplicate ifdefs

[Darrick J. Wong]

On Thu, Jan 18, 2024 at 01:33:25PM +0530, Shrikanth Hegde wrote:
> when a ifdef is used in the below manner, second one could be considered as
> duplicate.
> 
> ifdef DEFINE_A
> ...code block...
> ifdef DEFINE_A
> ...code block...
> endif
> ...code block...
> endif
> 
> There are few places in fs code where above pattern was seen.
> No functional change is intended here. It only aims to improve code
> readability.
> 
> Signed-off-by: Shrikanth Hegde 
> ---
>  fs/ntfs/inode.c| 2 --
>  fs/xfs/xfs_sysfs.c | 4 
>  2 files changed, 6 deletions(-)
> 
> diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
> index aba1e22db4e9..d2c8622d53d1 100644
> --- a/fs/ntfs/inode.c
> +++ b/fs/ntfs/inode.c
> @@ -2859,11 +2859,9 @@ int ntfs_truncate(struct inode *vi)
>   *
>   * See ntfs_truncate() description above for details.
>   */
> -#ifdef NTFS_RW
>  void ntfs_truncate_vfs(struct inode *vi) {
>   ntfs_truncate(vi);
>  }
> -#endif
> 
>  /**
>   * ntfs_setattr - called from notify_change() when an attribute is being 
> changed
> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> index 17485666b672..d2391eec37fe 100644
> --- a/fs/xfs/xfs_sysfs.c
> +++ b/fs/xfs/xfs_sysfs.c
> @@ -193,7 +193,6 @@ always_cow_show(
>  }
>  XFS_SYSFS_ATTR_RW(always_cow);
> 
> -#ifdef DEBUG
>  /*
>   * Override how many threads the parallel work queue is allowed to create.
>   * This has to be a debug-only global (instead of an errortag) because one of
> @@ -260,7 +259,6 @@ larp_show(
>   return snprintf(buf, PAGE_SIZE, "%d\n", xfs_globals.larp);
>  }
>  XFS_SYSFS_ATTR_RW(larp);
> -#endif /* DEBUG */
> 
>  STATIC ssize_t
>  bload_leaf_slack_store(
> @@ -319,10 +317,8 @@ static struct attribute *xfs_dbg_attrs[] = {
>   ATTR_LIST(log_recovery_delay),
>   ATTR_LIST(mount_delay),
>   ATTR_LIST(always_cow),
> -#ifdef DEBUG
>   ATTR_LIST(pwork_threads),
>   ATTR_LIST(larp),
> -#endif

The xfs part seems fine to me bcause I think some bot already
complained about this...

Reviewed-by: Darrick J. Wong 

--D

>   ATTR_LIST(bload_leaf_slack),
>   ATTR_LIST(bload_node_slack),
>   NULL,
> --
> 2.39.3
> 
> 

Re: [PATCH 86/87] fs: switch timespec64 fields in inode to discrete integers

[Darrick J. Wong]

On Thu, Sep 28, 2023 at 01:06:03PM -0400, Jeff Layton wrote:
> On Thu, 2023-09-28 at 11:48 -0400, Arnd Bergmann wrote:
> > On Thu, Sep 28, 2023, at 07:05, Jeff Layton wrote:
> > > This shaves 8 bytes off struct inode, according to pahole.
> > > 
> > > Signed-off-by: Jeff Layton 
> > 
> > FWIW, this is similar to the approach that Deepa suggested
> > back in 2016:
> > 
> > https://lore.kernel.org/lkml/1452144972-15802-3-git-send-email-deepa.ker...@gmail.com/
> > 
> > It was NaKed at the time because of the added complexity,
> > though it would have been much easier to do it then,
> > as we had to touch all the timespec references anyway.
> > 
> > The approach still seems ok to me, but I'm not sure it's worth
> > doing it now if we didn't do it then.
> > 
> 
> I remember seeing those patches go by. I don't remember that change
> being NaK'ed, but I wasn't paying close attention at the time 
> 
> Looking at it objectively now, I think it's worth it to recover 8 bytes
> per inode and open a 4 byte hole that Amir can use to grow the
> i_fsnotify_mask. We might even able to shave off another 12 bytes
> eventually if we can move to a single 64-bit word per timestamp. 

I don't think you can, since btrfs timestamps utilize s64 seconds
counting in both directions from the Unix epoch.  They also support ns
resolution:

struct btrfs_timespec {
__le64 sec;
__le32 nsec;
} __attribute__ ((__packed__));

--D

> It is a lot of churn though.
> -- 
> Jeff Layton 

Re: linux-next: build failure after merge of the mm tree

[Darrick J. Wong]

On Tue, Aug 22, 2023 at 02:34:06AM +0100, Matthew Wilcox wrote:
> On Tue, Aug 22, 2023 at 11:22:17AM +1000, Stephen Rothwell wrote:
> > Hi Matthew,
> > 
> > On Tue, 22 Aug 2023 02:11:44 +0100 Matthew Wilcox  
> > wrote:
> > >
> > > On Tue, Aug 22, 2023 at 09:55:37AM +1000, Stephen Rothwell wrote:
> > > > In file included from include/trace/trace_events.h:27,
> > > >  from include/trace/define_trace.h:102,
> > > >  from fs/xfs/xfs_trace.h:4428,
> > > >  from fs/xfs/xfs_trace.c:45:
> > > > include/linux/pgtable.h:8:25: error: initializer element is not constant
> > > > 8 | #define PMD_ORDER   (PMD_SHIFT - PAGE_SHIFT)  
> > > 
> > > Ummm.  PowerPC doesn't have a compile-time constant PMD size?
> > 
> > Yeah, you are not the first (or probably the last) to be caught by that.
> 
> I think this will do the trick.  Any comments?
> 
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 1904eaf7a2e9..d5a4e6c2dcd1 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -796,15 +796,6 @@ DEFINE_INODE_EVENT(xfs_inode_reclaiming);
>  DEFINE_INODE_EVENT(xfs_inode_set_need_inactive);
>  DEFINE_INODE_EVENT(xfs_inode_inactivating);
>  
> -/*
> - * ftrace's __print_symbolic requires that all enum values be wrapped in the
> - * TRACE_DEFINE_ENUM macro so that the enum value can be encoded in the 
> ftrace
> - * ring buffer.  Somehow this was only worth mentioning in the ftrace sample
> - * code.
> - */

Please leave this ^^^ comment, because the need for TRACE_DEFINE_ENUM to
make enums work in tracepoints is not at all obvious.

> -TRACE_DEFINE_ENUM(PMD_ORDER);
> -TRACE_DEFINE_ENUM(PUD_ORDER);
> -
>  TRACE_DEFINE_ENUM(XFS_REFC_DOMAIN_SHARED);
>  TRACE_DEFINE_ENUM(XFS_REFC_DOMAIN_COW);
>  
> @@ -823,13 +814,10 @@ TRACE_EVENT(xfs_filemap_fault,
>   __entry->order = order;
>   __entry->write_fault = write_fault;
>   ),
> - TP_printk("dev %d:%d ino 0x%llx %s write_fault %d",
> + TP_printk("dev %d:%d ino 0x%llx order:%u write_fault %d",

"order %u" to match the (non dev_t) style of the rest of the xfs
tracepoints.

--D

> MAJOR(__entry->dev), MINOR(__entry->dev),
> __entry->ino,
> -   __print_symbolic(__entry->order,
> - { 0,"PTE" },
> - { PMD_ORDER,"PMD" },
> - { PUD_ORDER,"PUD" }),
> +   __entry->order,
> __entry->write_fault)
>  )
>  
> 
> 

Re: [PATCH v1 1/5] treewide: use get_random_u32_below() instead of deprecated function

[Darrick J. Wong]

On Fri, Oct 21, 2022 at 09:43:59PM -0400, Jason A. Donenfeld wrote:
> This is a simple mechanical transformation done by:
> 
> @@
> expression E;
> @@
> - prandom_u32_max(E)
> + get_random_u32_below(E)
> 
> Signed-off-by: Jason A. Donenfeld 

I wish this patchset had included "random: use rejection sampling for
uniform bounded random integers" since I had to go pull it out of:

https://git.kernel.org/pub/scm/linux/kernel/git/crng/random.git/commit/?id=2a74fa8f83e2ab421e08462b639b703905c69249

Assuming that's the correct patch, the XFS changes look ok.
Though I'm trusting you that all the mathematics are correct since
that's /really/ not my department (more of a languages guy ;).

Acked-by: Darrick J. Wong 

--D

> ---
>  arch/arm/kernel/process.c |  2 +-
>  arch/arm64/kernel/process.c   |  2 +-
>  arch/loongarch/kernel/process.c   |  2 +-
>  arch/loongarch/kernel/vdso.c  |  2 +-
>  arch/mips/kernel/process.c|  2 +-
>  arch/mips/kernel/vdso.c   |  2 +-
>  arch/parisc/kernel/vdso.c |  2 +-
>  arch/powerpc/crypto/crc-vpmsum_test.c |  4 +-
>  arch/powerpc/kernel/process.c |  2 +-
>  arch/s390/kernel/process.c|  2 +-
>  arch/s390/kernel/vdso.c   |  2 +-
>  arch/sparc/vdso/vma.c |  2 +-
>  arch/um/kernel/process.c  |  2 +-
>  arch/x86/entry/vdso/vma.c |  2 +-
>  arch/x86/kernel/module.c  |  2 +-
>  arch/x86/kernel/process.c |  2 +-
>  arch/x86/mm/pat/cpa-test.c|  4 +-
>  crypto/rsa-pkcs1pad.c |  2 +-
>  crypto/testmgr.c  | 86 +--
>  drivers/block/drbd/drbd_receiver.c|  4 +-
>  drivers/bus/mhi/host/internal.h   |  2 +-
>  drivers/dma-buf/st-dma-fence-chain.c  |  6 +-
>  .../gpu/drm/i915/gem/i915_gem_execbuffer.c|  2 +-
>  .../drm/i915/gt/intel_execlists_submission.c  |  2 +-
>  drivers/gpu/drm/i915/intel_memory_region.c|  4 +-
>  drivers/infiniband/core/cma.c |  2 +-
>  drivers/infiniband/hw/cxgb4/id_table.c|  4 +-
>  drivers/infiniband/hw/hns/hns_roce_ah.c   |  4 +-
>  drivers/infiniband/ulp/rtrs/rtrs-clt.c|  2 +-
>  drivers/md/bcache/request.c   |  2 +-
>  drivers/media/common/v4l2-tpg/v4l2-tpg-core.c |  8 +-
>  .../media/test-drivers/vidtv/vidtv_demod.c|  8 +-
>  .../test-drivers/vivid/vivid-kthread-cap.c|  2 +-
>  .../test-drivers/vivid/vivid-kthread-out.c|  2 +-
>  .../media/test-drivers/vivid/vivid-radio-rx.c |  4 +-
>  .../media/test-drivers/vivid/vivid-sdr-cap.c  |  2 +-
>  .../test-drivers/vivid/vivid-touch-cap.c  |  2 +-
>  drivers/mmc/core/core.c   |  4 +-
>  drivers/mmc/host/dw_mmc.c |  2 +-
>  drivers/mtd/nand/raw/nandsim.c|  4 +-
>  drivers/mtd/tests/mtd_nandecctest.c   | 10 +--
>  drivers/mtd/tests/stresstest.c|  8 +-
>  drivers/mtd/ubi/debug.c   |  2 +-
>  drivers/mtd/ubi/debug.h   |  6 +-
>  drivers/net/ethernet/broadcom/cnic.c  |  2 +-
>  .../chelsio/inline_crypto/chtls/chtls_io.c|  4 +-
>  drivers/net/phy/at803x.c  |  2 +-
>  drivers/net/team/team_mode_random.c   |  2 +-
>  drivers/net/wireguard/selftest/allowedips.c   | 20 ++---
>  drivers/net/wireguard/timers.c|  4 +-
>  .../broadcom/brcm80211/brcmfmac/p2p.c |  2 +-
>  .../net/wireless/intel/iwlwifi/mvm/mac-ctxt.c |  2 +-
>  drivers/pci/p2pdma.c  |  2 +-
>  drivers/s390/scsi/zfcp_fc.c   |  2 +-
>  drivers/scsi/fcoe/fcoe_ctlr.c |  4 +-
>  drivers/scsi/qedi/qedi_main.c |  2 +-
>  drivers/scsi/scsi_debug.c |  6 +-
>  fs/ceph/inode.c   |  2 +-
>  fs/ceph/mdsmap.c  |  2 +-
>  fs/ext2/ialloc.c  |  2 +-
>  fs/ext4/ialloc.c  |  2 +-
>  fs/ext4/super.c   |  5 +-
>  fs/f2fs/gc.c  |  2 +-
>  fs/f2fs/segment.c |  8 +-
>  fs/ubifs/debug.c  |  8 +-
>  fs/ubifs/lpt_commit.c | 14 +--
>  fs/ubifs/tnc_commit.c |  2 +-
>  fs/xfs/libxfs/xfs_alloc.c |  2 +-
>  fs/xfs/libxfs/xfs_ialloc.c|  2 +-
>  fs/xfs/xfs_error.c|  2 +-

Re: [PATCH v4 4/6] treewide: use get_random_u32() when possible

[Darrick J. Wong]

On Fri, Oct 07, 2022 at 12:01:05PM -0600, Jason A. Donenfeld wrote:
> The prandom_u32() function has been a deprecated inline wrapper around
> get_random_u32() for several releases now, and compiles down to the
> exact same code. Replace the deprecated wrapper with a direct call to
> the real function. The same also applies to get_random_int(), which is
> just a wrapper around get_random_u32().
> 
> Reviewed-by: Kees Cook 
> Acked-by: Toke Høiland-Jørgensen  # for sch_cake
> Acked-by: Chuck Lever  # for nfsd
> Reviewed-by: Jan Kara  # for ext4
> Acked-by: Mika Westerberg  # for thunderbolt
> Signed-off-by: Jason A. Donenfeld 

For the XFS parts,
Acked-by: Darrick J. Wong 

--D

> ---
>  Documentation/networking/filter.rst|  2 +-
>  arch/parisc/kernel/process.c   |  2 +-
>  arch/parisc/kernel/sys_parisc.c|  4 ++--
>  arch/s390/mm/mmap.c|  2 +-
>  arch/x86/kernel/cpu/amd.c  |  2 +-
>  drivers/gpu/drm/i915/i915_gem_gtt.c|  6 +++---
>  drivers/gpu/drm/i915/selftests/i915_selftest.c |  2 +-
>  drivers/gpu/drm/selftests/test-drm_buddy.c |  2 +-
>  drivers/gpu/drm/selftests/test-drm_mm.c|  2 +-
>  drivers/infiniband/hw/cxgb4/cm.c   |  4 ++--
>  drivers/infiniband/hw/hfi1/tid_rdma.c  |  2 +-
>  drivers/infiniband/hw/mlx4/mad.c   |  2 +-
>  drivers/infiniband/ulp/ipoib/ipoib_cm.c|  2 +-
>  drivers/md/raid5-cache.c   |  2 +-
>  .../media/test-drivers/vivid/vivid-touch-cap.c |  4 ++--
>  drivers/misc/habanalabs/gaudi2/gaudi2.c|  2 +-
>  drivers/mtd/nand/raw/nandsim.c |  2 +-
>  drivers/net/bonding/bond_main.c|  2 +-
>  drivers/net/ethernet/broadcom/cnic.c   |  2 +-
>  .../chelsio/inline_crypto/chtls/chtls_cm.c |  2 +-
>  drivers/net/ethernet/rocker/rocker_main.c  |  6 +++---
>  .../wireless/broadcom/brcm80211/brcmfmac/pno.c |  2 +-
>  .../net/wireless/marvell/mwifiex/cfg80211.c|  4 ++--
>  .../net/wireless/microchip/wilc1000/cfg80211.c |  2 +-
>  .../net/wireless/quantenna/qtnfmac/cfg80211.c  |  2 +-
>  drivers/net/wireless/ti/wlcore/main.c  |  2 +-
>  drivers/nvme/common/auth.c |  2 +-
>  drivers/scsi/cxgbi/cxgb4i/cxgb4i.c |  4 ++--
>  drivers/target/iscsi/cxgbit/cxgbit_cm.c|  2 +-
>  drivers/thunderbolt/xdomain.c  |  2 +-
>  drivers/video/fbdev/uvesafb.c  |  2 +-
>  fs/exfat/inode.c   |  2 +-
>  fs/ext4/ialloc.c   |  2 +-
>  fs/ext4/ioctl.c|  4 ++--
>  fs/ext4/mmp.c  |  2 +-
>  fs/f2fs/namei.c|  2 +-
>  fs/fat/inode.c |  2 +-
>  fs/nfsd/nfs4state.c|  4 ++--
>  fs/ntfs3/fslog.c   |  6 +++---
>  fs/ubifs/journal.c |  2 +-
>  fs/xfs/libxfs/xfs_ialloc.c |  2 +-
>  fs/xfs/xfs_icache.c|  2 +-
>  fs/xfs/xfs_log.c   |  2 +-
>  include/net/netfilter/nf_queue.h   |  2 +-
>  include/net/red.h  |  2 +-
>  include/net/sock.h |  2 +-
>  kernel/bpf/bloom_filter.c  |  2 +-
>  kernel/bpf/core.c  |  2 +-
>  kernel/bpf/hashtab.c   |  2 +-
>  kernel/bpf/verifier.c  |  2 +-
>  kernel/kcsan/selftest.c|  2 +-
>  lib/random32.c |  2 +-
>  lib/reed_solomon/test_rslib.c  |  6 +++---
>  lib/test_fprobe.c  |  2 +-
>  lib/test_kprobes.c |  2 +-
>  lib/test_min_heap.c|  6 +++---
>  lib/test_rhashtable.c  |  6 +++---
>  mm/shmem.c |  2 +-
>  mm/slab.c  |  2 +-
>  net/802/garp.c |  2 +-
>  net/802/mrp.c  |  2 +-
>  net/core/pktgen.c  |  4 ++--
>  net/ipv4/route.c   |  2 +-
>  net/ipv4/tcp_cdg.c |  2 +-
>  net/ipv4/udp.c |  2 +-
>  net/ipv6/ip6_flowlabel.c   |  2 +-
>  net/ipv6/output_core.c |  2 +-
>  net/netfilter/ipvs/ip_vs_conn.c|  2 +-
>  net/netfilter/xt_statistic.c   |  2 +-
>  net/openv

Re: [PATCH v4 2/6] treewide: use prandom_u32_max() when possible

[Darrick J. Wong]

On Fri, Oct 07, 2022 at 12:01:03PM -0600, Jason A. Donenfeld wrote:
> Rather than incurring a division or requesting too many random bytes for
> the given range, use the prandom_u32_max() function, which only takes
> the minimum required bytes from the RNG and avoids divisions.
> 
> Reviewed-by: Kees Cook 
> Reviewed-by: KP Singh 
> Reviewed-by: Christoph Böhmwalder  # for 
> drbd
> Reviewed-by: Jan Kara  # for ext2, ext4, and sbitmap
> Signed-off-by: Jason A. Donenfeld 
> ---



> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index e2bdf089c0a3..6261599bb389 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -1520,7 +1520,7 @@ xfs_alloc_ag_vextent_lastblock(
>  
>  #ifdef DEBUG
>   /* Randomly don't execute the first algorithm. */
> - if (prandom_u32() & 1)
> + if (prandom_u32_max(2))

I wonder if these usecases (picking 0 or 1 randomly) ought to have a
trivial wrapper to make it more obvious that we want boolean semantics:

static inline bool prandom_bool(void)
{
return prandom_u32_max(2);
}

if (prandom_bool())
use_crazy_algorithm(...);

But this translation change looks correct to me, so for the XFS parts:
Acked-by: Darrick J. Wong 

--D


>   return 0;
>  #endif
>  
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 6cdfd64bc56b..7838b31126e2 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -636,7 +636,7 @@ xfs_ialloc_ag_alloc(
>   /* randomly do sparse inode allocations */
>   if (xfs_has_sparseinodes(tp->t_mountp) &&
>   igeo->ialloc_min_blks < igeo->ialloc_blks)
> - do_sparse = prandom_u32() & 1;
> + do_sparse = prandom_u32_max(2);
>  #endif
>  
>   /*
> diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h
> index 4b71a96190a8..66ee9b4b7925 100644
> --- a/include/linux/nodemask.h
> +++ b/include/linux/nodemask.h
> @@ -509,7 +509,7 @@ static inline int node_random(const nodemask_t *maskp)
>   w = nodes_weight(*maskp);
>   if (w)
>   bit = bitmap_ord_to_pos(maskp->bits,
> - get_random_int() % w, MAX_NUMNODES);
> + prandom_u32_max(w), MAX_NUMNODES);
>   return bit;
>  #else
>   return 0;
> diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c
> index e6a31c927b06..a72a2c16066e 100644
> --- a/lib/cmdline_kunit.c
> +++ b/lib/cmdline_kunit.c
> @@ -76,7 +76,7 @@ static void cmdline_test_lead_int(struct kunit *test)
>   int rc = cmdline_test_values[i];
>   int offset;
>  
> - sprintf(in, "%u%s", prandom_u32_max(256), str);
> + sprintf(in, "%u%s", get_random_int() % 256, str);
>   /* Only first '-' after the number will advance the pointer */
>   offset = strlen(in) - strlen(str) + !!(rc == 2);
>   cmdline_do_one_test(test, in, rc, offset);
> @@ -94,7 +94,7 @@ static void cmdline_test_tail_int(struct kunit *test)
>   int rc = strcmp(str, "") ? (strcmp(str, "-") ? 0 : 1) : 1;
>   int offset;
>  
> - sprintf(in, "%s%u", str, prandom_u32_max(256));
> + sprintf(in, "%s%u", str, get_random_int() % 256);
>   /*
>* Only first and leading '-' not followed by integer
>* will advance the pointer.
> diff --git a/lib/kobject.c b/lib/kobject.c
> index 5f0e71ab292c..a0b2dbfcfa23 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -694,7 +694,7 @@ static void kobject_release(struct kref *kref)
>  {
>   struct kobject *kobj = container_of(kref, struct kobject, kref);
>  #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
> - unsigned long delay = HZ + HZ * (get_random_int() & 0x3);
> + unsigned long delay = HZ + HZ * prandom_u32_max(4);
>   pr_info("kobject: '%s' (%p): %s, parent %p (delayed %ld)\n",
>kobject_name(kobj), kobj, __func__, kobj->parent, delay);
>   INIT_DELAYED_WORK(>release, kobject_delayed_cleanup);
> diff --git a/lib/reed_solomon/test_rslib.c b/lib/reed_solomon/test_rslib.c
> index 6faf9c9a6215..4d241bdc88aa 100644
> --- a/lib/reed_solomon/test_rslib.c
> +++ b/lib/reed_solomon/test_rslib.c
> @@ -199,7 +199,7 @@ static int get_rcw_we(struct rs_control *rs, struct 
> wspace *ws,
>  
>   derrlocs[i] = errloc;
>  
> - if (ewsc && (prandom_u32() & 1)) {
> + if (ewsc && prandom_u32_max(2)) {
>   /* Erasure with the symbol intact */
>  

Re: [PATCH v4 1/6] treewide: use prandom_u32_max() when possible, mechanically

[Darrick J. Wong]

On Fri, Oct 07, 2022 at 12:01:02PM -0600, Jason A. Donenfeld wrote:
> Rather than incurring a division or requesting too many random bytes for
> the given range, use the prandom_u32_max() function, which only takes
> the minimum required bytes from the RNG and avoids divisions. This was
> done mechanically with these coccinelle scripts:
> 
> @no_modulo@
> expression E;
> @@
> -   (prandom_u32() % (E))
> +   prandom_u32_max(E)
> @no_modulo@
> expression E;
> @@
> -   (get_random_u32() % (E))
> +   prandom_u32_max(E)
> @no_modulo@
> expression E;
> @@
> -   (get_random_int() % (E))
> +   prandom_u32_max(E)
> 
> Reviewed-by: Kees Cook 
> Reviewed-by: KP Singh 
> Reviewed-by: Jan Kara  # for ext4 and sbitmap
> Acked-by: Ulf Hansson  # for mmc
> Signed-off-by: Jason A. Donenfeld 

For the XFS part,
Acked-by: Darrick J. Wong 

--D

> ---
>  arch/arm/kernel/process.c |  2 +-
>  arch/s390/kernel/vdso.c   |  2 +-
>  arch/um/kernel/process.c  |  2 +-
>  arch/x86/entry/vdso/vma.c |  2 +-
>  arch/x86/kernel/module.c  |  2 +-
>  arch/x86/kernel/process.c |  2 +-
>  arch/x86/mm/pat/cpa-test.c|  4 +-
>  crypto/testmgr.c  | 86 +--
>  .../gpu/drm/i915/gem/i915_gem_execbuffer.c|  2 +-
>  drivers/infiniband/core/cma.c |  2 +-
>  drivers/infiniband/hw/cxgb4/id_table.c|  4 +-
>  drivers/infiniband/hw/hns/hns_roce_ah.c   |  5 +-
>  drivers/infiniband/ulp/rtrs/rtrs-clt.c|  3 +-
>  .../test-drivers/vivid/vivid-touch-cap.c  |  2 +-
>  drivers/mmc/core/core.c   |  4 +-
>  drivers/mmc/host/dw_mmc.c |  2 +-
>  drivers/mtd/nand/raw/nandsim.c|  4 +-
>  drivers/mtd/tests/mtd_nandecctest.c   | 10 +--
>  drivers/mtd/ubi/debug.c   |  2 +-
>  .../chelsio/inline_crypto/chtls/chtls_io.c|  4 +-
>  drivers/net/hamradio/baycom_epp.c |  2 +-
>  drivers/net/hamradio/hdlcdrv.c|  2 +-
>  drivers/net/hamradio/yam.c|  2 +-
>  drivers/net/phy/at803x.c  |  2 +-
>  .../broadcom/brcm80211/brcmfmac/p2p.c |  2 +-
>  .../net/wireless/intel/iwlwifi/mvm/mac-ctxt.c |  2 +-
>  drivers/scsi/fcoe/fcoe_ctlr.c |  4 +-
>  drivers/scsi/qedi/qedi_main.c |  2 +-
>  fs/ceph/inode.c   |  2 +-
>  fs/ceph/mdsmap.c  |  2 +-
>  fs/ext4/super.c   |  7 +-
>  fs/f2fs/gc.c  |  2 +-
>  fs/f2fs/segment.c |  8 +-
>  fs/ubifs/debug.c  |  8 +-
>  fs/ubifs/lpt_commit.c | 12 +--
>  fs/xfs/xfs_error.c|  2 +-
>  kernel/bpf/core.c |  4 +-
>  kernel/locking/test-ww_mutex.c|  4 +-
>  kernel/time/clocksource.c |  2 +-
>  lib/cmdline_kunit.c   |  4 +-
>  lib/fault-inject.c|  2 +-
>  lib/find_bit_benchmark.c  |  4 +-
>  lib/reed_solomon/test_rslib.c |  4 +-
>  lib/sbitmap.c |  2 +-
>  lib/test-string_helpers.c |  2 +-
>  lib/test_hexdump.c| 10 +--
>  lib/test_kasan.c  |  6 +-
>  lib/test_list_sort.c  |  2 +-
>  mm/migrate.c  |  2 +-
>  mm/slub.c |  2 +-
>  net/ceph/mon_client.c |  2 +-
>  net/ceph/osd_client.c |  2 +-
>  net/core/neighbour.c  |  2 +-
>  net/core/pktgen.c | 39 -
>  net/core/stream.c |  2 +-
>  net/ipv4/igmp.c   |  6 +-
>  net/ipv4/inet_connection_sock.c   |  2 +-
>  net/ipv6/addrconf.c   |  8 +-
>  net/ipv6/mcast.c  | 10 +--
>  net/netfilter/ipvs/ip_vs_twos.c   |  4 +-
>  net/packet/af_packet.c|  2 +-
>  net/sched/act_gact.c  |  2 +-
>  net/sched/act_sample.c|  2 +-
>  net/sched/sch_netem.c |  4 +-
>  net/sctp/socket.c |  2 +-
>  net/tipc/socket.c |  2 +-

Re: [next-20220215] WARNING at fs/iomap/buffered-io.c:75 with xfstests

[Darrick J. Wong]

[adding fsdevel to this party, since iomap is core code, not just XFS...]

On Wed, Feb 16, 2022 at 12:55:02PM +0530, Sachin Sant wrote:
> While running xfstests on IBM Power10 logical partition (LPAR) booted
> with 5.17.0-rc4-next-20220215 following warning was seen:
> 
> [ 2547.384210] xfs filesystem being mounted at /mnt/scratch supports 
> timestamps until 2038 (0x7fff)
> [ 2547.389021] XFS (dm-0): metadata I/O error in "xfs_imap_to_bp+0x64/0x98 
> [xfs]" at daddr 0x2bc2c0 len 32 error 5
> [ 2547.389020] XFS (dm-0): metadata I/O error in "xfs_imap_to_bp+0x64/0x98 
> [xfs]" at daddr 0x15cede0 len 32 error 5
> [ 2547.389026] XFS (dm-0): metadata I/O error in 
> "xfs_btree_read_buf_block.constprop.29+0xd0/0x110 [xfs]" at daddr 0xc60 len 8 
> error 5
> [ 2547.389032] XFS (dm-0): metadata I/O error in 
> "xfs_btree_read_buf_block.constprop.29+0xd0/0x110 [xfs]" at daddr 0x3bffa30 
> len 8 error 5
> [ 2547.389120] XFS (dm-0): log I/O error -5
> [ 2547.389135] XFS (dm-0): Metadata I/O Error (0x1) detected at 
> xfs_trans_read_buf_map+0x31c/0x368 [xfs] (fs/xfs/xfs_trans_buf.c:296).  
> Shutting down filesystem.

Hmm, the filesystem went down...

> [ 2547.389195] XFS (dm-0): Please unmount the filesystem and rectify the 
> problem(s)
> [ 2547.662818] [ cut here ]
> [ 2547.662832] WARNING: CPU: 24 PID: 2463718 at fs/iomap/buffered-io.c:75 
> iomap_page_release+0x1b0/0x1e0

...and I think this is complaining about this debugging test in
iomap_page_release:

WARN_ON_ONCE(bitmap_full(iop->uptodate, nr_blocks) !=
folio_test_uptodate(folio));

which checks that we're not releasing or invalidating a page that's
partially uptodate on a blocksize < pagesize filesystem (or so I gather
from "POWER10 LPAR" (64k pages?) and "XFS" (4k blocks?))...

> [ 2547.662840] Modules linked in: dm_thin_pool dm_persistent_data 
> dm_bio_prison dm_snapshot dm_bufio loop dm_flakey xfs libcrc32c dm_mod rfkill 
> bonding sunrpc pseries_rng xts vmx_crypto uio_pdrv_genirq uio sch_fq_codel 
> ext4 mbcache jbd2 sd_mod t10_pi sg ibmvscsi ibmveth scsi_transport_srp fuse 
> [last unloaded: scsi_debug]
> [ 2547.662866] CPU: 24 PID: 2463718 Comm: umount Not tainted 
> 5.17.0-rc4-next-20220215 #1
> [ 2547.662871] NIP:  c0565b80 LR: c0565aa0 CTR: 
> c0565e40
> [ 2547.662874] REGS: cf0035b0 TRAP: 0700   Not tainted  
> (5.17.0-rc4-next-20220215)
> [ 2547.662877] MSR:  8282b033   CR: 
> 44000228  XER: 2004
> [ 2547.662885] CFAR: c0565ac8 IRQMASK: 0 
> [ 2547.662885] GPR00: c037dd3c cf003850 c2a03300 
> 0001 
> [ 2547.662885] GPR04: 0010 0001 c000df6e7bc0 
> 0001 
> [ 2547.662885] GPR08: fffe  0010 
> 0228 
> [ 2547.662885] GPR12: c0565e40 c00abfcfe680  
>  
> [ 2547.662885] GPR16:    
>  
> [ 2547.662885] GPR20:    
> fffe 
> [ 2547.662885] GPR24:   cf0038e0 
> c000243db278 
> [ 2547.662885] GPR28: 0012 c000df6e7ae0 0010 
> c00c0240c100 
> [ 2547.662920] NIP [c0565b80] iomap_page_release+0x1b0/0x1e0
> [ 2547.662924] LR [c0565aa0] iomap_page_release+0xd0/0x1e0
> [ 2547.662927] Call Trace:
> [ 2547.662928] [cf003850] [cf003890] 0xcf003890 
> (unreliable)
> [ 2547.662932] [cf003890] [c037dd3c] 
> truncate_cleanup_folio+0x7c/0x140
> [ 2547.662937] [cf0038c0] [c037f068] 
> truncate_inode_pages_range+0x148/0x5e0
> [ 2547.662942] [cf003a40] [c04c2058] evict+0x248/0x270
> [ 2547.662946] [cf003a80] [c04c20fc] dispose_list+0x7c/0xb0
> [ 2547.662951] [cf003ac0] [c04c2314] evict_inodes+0x1e4/0x300
> [ 2547.662955] [cf003b60] [c04922d0] 
> generic_shutdown_super+0x70/0x1e0

...but in this particular case, we're dumping pages (and inodes) as part
of unmounting the filesystem, in which case we've already flushed dirty
data to disk, because sync_filesystem gets called before evict_inodes
here...

> [ 2547.662959] [cf003bd0] [c0492518] 
> kill_block_super+0x38/0x90
> [ 2547.662964] [cf003c00] [c04926e8] 
> deactivate_locked_super+0x78/0xf0
> [ 2547.662968] [cf003c30] [c04cde9c] cleanup_mnt+0xfc/0x1c0
> [ 2547.662972] [cf003c80] [c0189448] task_work_run+0xf8/0x170
> [ 2547.662976] [cf003cd0] [c0021b94] 
> do_notify_resume+0x434/0x480
> [ 2547.662981] [cf003d80] [c00338b0] 
> interrupt_exit_user_prepare_main+0x1a0/0x260
> [ 2547.662985] [cf003de0] [c0033d74] 
> syscall_exit_prepare+0x74/0x150
> [ 2547.662989] [cf003e10] [c000c658] 
> 

Re: [bug] userspace hitting sporadic SIGBUS on xfs (Power9, ppc64le), v4.19 and later

[Darrick J. Wong]

On Tue, Dec 03, 2019 at 09:35:28AM -0500, Jan Stancek wrote:
> 
> - Original Message -
> > On Tue, Dec 03, 2019 at 07:50:39AM -0500, Jan Stancek wrote:
> > > My theory is that there's a race in iomap. There appear to be
> > > interleaved calls to iomap_set_range_uptodate() for same page
> > > with varying offset and length. Each call sees bitmap as _not_
> > > entirely "uptodate" and hence doesn't call SetPageUptodate().
> > > Even though each bit in bitmap ends up uptodate by the time
> > > all calls finish.
> > 
> > Weird.  That should be prevented by the page lock that all callers
> > of iomap_set_range_uptodate.  But in case I miss something, does
> > the patch below trigger?  If not it is not jut a race, but might
> > be some weird ordering problem with the bitops, especially if it
> > only triggers on ppc, which is very weakly ordered.
> > 
> > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> > index d33c7bc5ee92..25e942c71590 100644
> > --- a/fs/iomap/buffered-io.c
> > +++ b/fs/iomap/buffered-io.c
> > @@ -148,6 +148,8 @@ iomap_set_range_uptodate(struct page *page, unsigned 
> > off,
> > unsigned len)
> > unsigned int i;
> > bool uptodate = true;
> >  
> > +   WARN_ON_ONCE(!PageLocked(page));
> > +
> > if (iop) {
> > for (i = 0; i < PAGE_SIZE / i_blocksize(inode); i++) {
> > if (i >= first && i <= last)
> > 
> 
> Hit it pretty quick this time:
> 
> # uptime
>  09:27:42 up 22 min,  2 users,  load average: 0.09, 13.38, 26.18
> 
> # /mnt/testarea/ltp/testcases/bin/genbessel   
>   
> 
> Bus error (core dumped)
> 
> # dmesg | grep -i -e warn -e call 
>   
> 
> [0.00] dt-cpu-ftrs: not enabling: system-call-vectored (disabled or 
> unsupported by kernel)
> [0.00] random: get_random_u64 called from 
> cache_random_seq_create+0x98/0x1e0 with crng_init=0
> [0.00] rcu: Offload RCU callbacks from CPUs: (none).
> [5.312075] megaraid_sas 0031:01:00.0: megasas_disable_intr_fusion is 
> called outbound_intr_mask:0x4009
> [5.357307] megaraid_sas 0031:01:00.0: megasas_disable_intr_fusion is 
> called outbound_intr_mask:0x4009
> [5.485126] megaraid_sas 0031:01:00.0: megasas_enable_intr_fusion is 
> called outbound_intr_mask:0x4000
> 
> So, extra WARN_ON_ONCE applied on top of v5.4-8836-g81b6b96475ac
> did not trigger.
> 
> Is it possible for iomap code to submit multiple bio-s for same
> locked page and then receive callbacks in parallel?

Yes, if (say) you have 64k pages on a 4k-block filesystem and the extent
mapping for all 16 blocks aren't contiguous, then iomap will issue
separate bios for each physical fragment it finds.  iomap will call
submit_bio on those bios whenever it thinks it's done filling the bio,
so you can indeed get multiple callbacks in parallel.

--D

Re: [PATCH] xfs: introduce "metasync" api to sync metadata to fsblock

[Darrick J. Wong]

On Tue, Oct 15, 2019 at 09:10:04AM -0400, Theodore Y. Ts'o wrote:
> On Tue, Oct 15, 2019 at 01:01:02AM -0700, Christoph Hellwig wrote:
> > On Mon, Oct 14, 2019 at 03:09:48PM -0500, Eric Sandeen wrote:
> > > We're in agreement here.  ;)  I only worry about implementing things like 
> > > this
> > > which sound like guarantees, but aren't, and end up encouraging bad 
> > > behavior
> > > or promoting misconceptions.
> > > 
> > > More and more, I think we should reconsider Darrick's "bootfs" (ext2 by 
> > > another
> > > name, but with extra-sync-iness) proposal...
> > 
> > Having a separate simple file system for the boot loader makes a lot of
> > sense.  Note that vfat of EFI is the best choice, but at least it is
> > something.  SysV Unix from the 90s actually had a special file system just
> > for that, and fs/bfs/ in Linux supports that.  So this isn't really a new
> > thing either.
> 
> Did you mean to say "vfaat of EFI isn't the best choice"?
> 
> If we were going to be doing something like "bootfs", what sort of
> semantics would be sufficient?  Is doing an implied fsync() on every
> close(2) enough, or do we need to do something even more conservative?

I'm assuming you'd also want to make sure the journal checkpoints as
part of fsync, right?  Aside from being an April Fools joke, bootfs[1]
does implement the semantics I needed to fix all the complaining about
grub being broken. 8-)

Granted there's also the systemd bootloader spec[2] which says
FAT{16,32}...

[1] https://lore.kernel.org/linux-fsdevel/20190401070001.GJ1173@magnolia/
[2] https://systemd.io/BOOT_LOADER_SPECIFICATION.html

--D

>- Ted

Re: [PATCH] xfs: introduce "metasync" api to sync metadata to fsblock

[Darrick J. Wong]

On Sun, Oct 13, 2019 at 10:37:00PM +0800, Pingfan Liu wrote:
> When using fadump (fireware assist dump) mode on powerpc, a mismatch
> between grub xfs driver and kernel xfs driver has been obsevered.  Note:
> fadump boots up in the following sequence: fireware -> grub reads kernel
> and initramfs -> kernel boots.
> 
> The process to reproduce this mismatch:
>   - On powerpc, boot kernel with fadump=on and edit /etc/kdump.conf.
>   - Replacing "path /var/crash" with "path /var/crashnew", then, "kdumpctl
> restart" to rebuild the initramfs. Detail about the rebuilding looks
> like: mkdumprd /boot/initramfs-`uname -r`.img.tmp;
>   mv /boot/initramfs-`uname -r`.img.tmp /boot/initramfs-`uname -r`.img
>   sync
>   - "echo c >/proc/sysrq-trigger".
> 
> The result:
> The dump image will not be saved under /var/crashnew/* as expected, but
> still saved under /var/crash.
> 
> The root cause:
> As Eric pointed out that on xfs, 'sync' ensures the consistency by writing
> back metadata to xlog, but not necessary to fsblock. This raises issue if
> grub can not replay the xlog before accessing the xfs files. Since the
> above dir entry of initramfs should be saved as inline data with xfs_inode,
> so xfs_fs_sync_fs() does not guarantee it written to fsblock.
> 
> umount can be used to write metadata fsblock, but the filesystem can not be
> umounted if still in use.
> 
> There are two ways to fix this mismatch, either grub or xfs. It may be
> easier to do this in xfs side by introducing an interface to flush metadata
> to fsblock explicitly.
> 
> With this patch, metadata can be written to fsblock by:
>   # update AIL
>   sync
>   # new introduced interface to flush metadata to fsblock
>   mount -o remount,metasync mountpoint

I think this ought to be an ioctl or some sort of generic call since the
jbd2 filesystems (ext3, ext4, ocfs2) suffer from the same "$BOOTLOADER
is too dumb to recover logs but still wants to write to the fs"
checkpointing problem.

(Or maybe we should just put all that stuff in a vfat filesystem, I
don't know...)

--D

> Signed-off-by: Pingfan Liu 
> Cc: "Darrick J. Wong" 
> Cc: Dave Chinner 
> Cc: Eric Sandeen 
> Cc: Hari Bathini 
> Cc: linuxppc-dev@lists.ozlabs.org
> To: linux-...@vger.kernel.org
> ---
>  fs/xfs/xfs_mount.h  |  1 +
>  fs/xfs/xfs_super.c  | 15 ++-
>  fs/xfs/xfs_trans.h  |  2 ++
>  fs/xfs/xfs_trans_ail.c  | 26 +-
>  fs/xfs/xfs_trans_priv.h |  1 +
>  5 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index fdb60e0..85f32e6 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -243,6 +243,7 @@ typedef struct xfs_mount {
>  #define XFS_MOUNT_FILESTREAMS(1ULL << 24)/* enable the 
> filestreams
>  allocator */
>  #define XFS_MOUNT_NOATTR2(1ULL << 25)/* disable use of attr2 format 
> */
> +#define XFS_MOUNT_METASYNC   (1ull << 26)/* write meta to fsblock */
>  
>  #define XFS_MOUNT_DAX(1ULL << 62)/* TEST ONLY! */
>  
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 8d1df9f..41df810 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -59,7 +59,7 @@ enum {
>   Opt_filestreams, Opt_quota, Opt_noquota, Opt_usrquota, Opt_grpquota,
>   Opt_prjquota, Opt_uquota, Opt_gquota, Opt_pquota,
>   Opt_uqnoenforce, Opt_gqnoenforce, Opt_pqnoenforce, Opt_qnoenforce,
> - Opt_discard, Opt_nodiscard, Opt_dax, Opt_err,
> + Opt_discard, Opt_nodiscard, Opt_dax, Opt_metasync, Opt_err
>  };
>  
>  static const match_table_t tokens = {
> @@ -106,6 +106,7 @@ static const match_table_t tokens = {
>   {Opt_discard,   "discard"}, /* Discard unused blocks */
>   {Opt_nodiscard, "nodiscard"},   /* Do not discard unused blocks */
>   {Opt_dax,   "dax"}, /* Enable direct access to bdev pages */
> + {Opt_metasync,  "metasync"},/* one shot to write meta to fsblock */
>   {Opt_err,   NULL},
>  };
>  
> @@ -338,6 +339,9 @@ xfs_parseargs(
>   mp->m_flags |= XFS_MOUNT_DAX;
>   break;
>  #endif
> + case Opt_metasync:
> + mp->m_flags |= XFS_MOUNT_METASYNC;
> + break;
>   default:
>   xfs_warn(mp, "unknown mount option [%s].", p);
>   return -EINVAL;
> @@ -1259,6 +1263,9 @@ xfs_fs_remount(
> 

Re: [trivial PATCH] treewide: Align function definition open/close braces

[Darrick J. Wong]

4
> --- a/drivers/message/fusion/mptsas.c
> +++ b/drivers/message/fusion/mptsas.c
> @@ -2968,7 +2968,7 @@ mptsas_exp_repmanufacture_info(MPT_ADAPTER *ioc,
>   mutex_unlock(>sas_mgmt.mutex);
>  out:
>   return ret;
> - }
> +}
>  
>  static void
>  mptsas_parse_device_info(struct sas_identify *identify,
> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c 
> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
> index 3dd973475125..0ea141ece19e 100644
> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
> @@ -603,7 +603,7 @@ static struct uni_table_desc *nx_get_table_desc(const u8 
> *unirom, int section)
>  
>  static int
>  netxen_nic_validate_header(struct netxen_adapter *adapter)
> - {
> +{
>   const u8 *unirom = adapter->fw->data;
>   struct uni_table_desc *directory = (struct uni_table_desc *) [0];
>   u32 fw_file_size = adapter->fw->size;
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c 
> b/drivers/net/wireless/ath/ath9k/xmit.c
> index bd438062a6db..baedc7186b10 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -196,7 +196,7 @@ ath_tid_pull(struct ath_atx_tid *tid)
>   }
>  
>   return skb;
> - }
> +}
>  
>  static struct sk_buff *ath_tid_dequeue(struct ath_atx_tid *tid)
>  {
> diff --git a/drivers/platform/x86/eeepc-laptop.c 
> b/drivers/platform/x86/eeepc-laptop.c
> index 5a681962899c..4c38904a8a32 100644
> --- a/drivers/platform/x86/eeepc-laptop.c
> +++ b/drivers/platform/x86/eeepc-laptop.c
> @@ -492,7 +492,7 @@ static void eeepc_platform_exit(struct eeepc_laptop 
> *eeepc)
>   * potentially bad time, such as a timer interrupt.
>   */
>  static void tpd_led_update(struct work_struct *work)
> - {
> +{
>   struct eeepc_laptop *eeepc;
>  
>   eeepc = container_of(work, struct eeepc_laptop, tpd_led_work);
> diff --git a/drivers/rtc/rtc-ab-b5ze-s3.c b/drivers/rtc/rtc-ab-b5ze-s3.c
> index a319bf1e49de..ef5c16dfabfa 100644
> --- a/drivers/rtc/rtc-ab-b5ze-s3.c
> +++ b/drivers/rtc/rtc-ab-b5ze-s3.c
> @@ -648,7 +648,7 @@ static int abb5zes3_rtc_set_alarm(struct device *dev, 
> struct rtc_wkalrm *alarm)
>   ret);
>  
>   return ret;
> - }
> +}
>  
>  /* Enable or disable battery low irq generation */
>  static inline int _abb5zes3_rtc_battery_low_irq_enable(struct regmap *regmap,
> diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c
> index fd172b0890d3..a00d822e3142 100644
> --- a/drivers/scsi/dpt_i2o.c
> +++ b/drivers/scsi/dpt_i2o.c
> @@ -3524,7 +3524,7 @@ static int adpt_i2o_systab_send(adpt_hba* pHba)
>  #endif
>  
>   return ret; 
> - }
> +}
>  
>  
>  
> /*
> diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c 
> b/drivers/scsi/sym53c8xx_2/sym_glue.c
> index 791a2182de53..7320d5fe4cbc 100644
> --- a/drivers/scsi/sym53c8xx_2/sym_glue.c
> +++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
> @@ -1393,7 +1393,7 @@ static struct Scsi_Host *sym_attach(struct 
> scsi_host_template *tpnt, int unit,
>   scsi_host_put(shost);
>  
>   return NULL;
> - }
> +}
>  
>  
>  /*
> diff --git a/fs/locks.c b/fs/locks.c
> index 21b4dfa289ee..d2399d001afe 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -559,7 +559,7 @@ static const struct lock_manager_operations 
> lease_manager_ops = {
>   * Initialize a lease, use the default lock manager operations
>   */
>  static int lease_init(struct file *filp, long type, struct file_lock *fl)
> - {
> +{
>   if (assign_type(fl, type) != 0)
>   return -EINVAL;
>  
> diff --git a/fs/ocfs2/stack_user.c b/fs/ocfs2/stack_user.c
> index dae9eb7c441e..d2fb97b173da 100644
> --- a/fs/ocfs2/stack_user.c
> +++ b/fs/ocfs2/stack_user.c
> @@ -398,7 +398,7 @@ static int ocfs2_control_do_setnode_msg(struct file *file,
>  
>  static int ocfs2_control_do_setversion_msg(struct file *file,
>  struct ocfs2_control_message_setv 
> *msg)
> - {
> +{
>   long major, minor;
>   char *ptr = NULL;
>   struct ocfs2_control_private *p = file->private_data;
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 0da80019a917..217108f765d5 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2401,7 +2401,7 @@ static bool
>  xfs_agf_verify(
>   struct xfs_mount *mp,
>   struct xfs_buf  *bp)
> - {
> +{
>   struct xfs_agf  *agf = XFS_BUF_TO_AGF(bp);
>  
>   if (xfs_

Re: [linux-next][XFS][trinity] WARNING: CPU: 32 PID: 31369 at fs/iomap.c:993

[Darrick J. Wong]

On Mon, Sep 18, 2017 at 05:00:58PM -0500, Eric Sandeen wrote:
> On 9/18/17 4:31 PM, Dave Chinner wrote:
> > On Mon, Sep 18, 2017 at 09:28:55AM -0600, Jens Axboe wrote:
> >> On 09/18/2017 09:27 AM, Christoph Hellwig wrote:
> >>> On Mon, Sep 18, 2017 at 08:26:05PM +0530, Abdul Haleem wrote:
>  Hi,
> 
>  A warning is triggered from:
> 
>  file fs/iomap.c in function iomap_dio_rw
> 
>  if (ret)
>  goto out_free_dio;
> 
>  ret = invalidate_inode_pages2_range(mapping,
>  start >> PAGE_SHIFT, end >> PAGE_SHIFT);
> >>  WARN_ON_ONCE(ret);
>  ret = 0;
> 
>  inode_dio_begin(inode);
> >>>
> >>> This is expected and an indication of a problematic workload - which
> >>> may be triggered by a fuzzer.
> >>
> >> If it's expected, why don't we kill the WARN_ON_ONCE()? I get it all
> >> the time running xfstests as well.
> > 
> > Because when a user reports a data corruption, the only evidence we
> > have that they are running an app that does something stupid is this
> > warning in their syslogs.  Tracepoints are not useful for replacing
> > warnings about data corruption vectors being triggered.
> 
> Is the full WARN_ON spew really helpful to us, though?  Certainly
> the user has no idea what it means, and will come away terrified
> but none the wiser.
> 
> Would a more informative printk_once() still give us the evidence
> without the ZOMG I THINK I OOPSED that a WARN_ON produces?  Or do we 
> want/need the backtrace?

Maybe we could state a little more directly what's going on:

if (err)
printk_once(KERN_INFO "Urk, collision detected between direct IO and 
page cache, YHL. HAND.\n"); ?

8-)

--D

> 
> -Eric
> 
> > It needs to be on by default, bu tI'm sure we can wrap it with
> > something like an xfs_alert_tag() type of construct so the tag can
> > be set in /proc/fs/xfs/panic_mask to suppress it if testers so
> > desire.
> > 
> > Cheers,
> > 
> > Dave.
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-next" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel bug in "Drop WIMG in favour of new constants"?

[Darrick J. Wong]

On Thu, Jun 16, 2016 at 08:01:55PM +0530, Aneesh Kumar K.V wrote:
> "Aneesh Kumar K.V" <aneesh.ku...@linux.vnet.ibm.com> writes:
> 
> > "Aneesh Kumar K.V" <aneesh.ku...@linux.vnet.ibm.com> writes:
> >
> >> "Aneesh Kumar K.V" <aneesh.ku...@linux.vnet.ibm.com> writes:
> >>
> >>> "Darrick J. Wong" <darrick.w...@oracle.com> writes:
> >>>
> >>>> On Thu, Jun 16, 2016 at 03:23:47PM +1000, Michael Ellerman wrote:
> >>>>> On Wed, 2016-06-15 at 21:33 -0700, Darrick J. Wong wrote:
> >>>>> 
> >>>>> > Hi Aneesh,
> >>>>> > 
> >>>>> > I noticed when trying out 4.7-rc3 on qemu-2.5 that the kernel no 
> >>>>> > longer
> >>>>> > boots.  4.6 booted just fine, so I bisected the kernel to the commit
> >>>>> > 30bda41aba4efb2370c97e2cbe7385de93ccc372, which is "powerpc/mm: Drop 
> >>>>> > WIMG in
> >>>>> > favour of new constants".  The changelog suggests that the KVM 
> >>>>> > changes need
> >>>>> > closer review, and here's an actual crash:
> >>>>> > 
> >>>>> > (I can send libvirt's machine xml, .config, and full dmesg if that 
> >>>>> > helps.)
> >>>>> 
> >>>>> Yes please.
> >>>>> 
> >>>>> I'm successfully booting 4.7-rc's on qemu (2.5.0 (Debian 
> >>>>> 1:2.5+dfsg-5ubuntu10.1)).
> >>>>
> >>>> Ok, see attached.  I also sent along the dpkg --status output for qemu
> >>>> and qemu-slof; looks like we're running the same Ubuntu packages...
> >>>>
> >>>> ...my host kernel is 4.6.0 on x64.
> >>>
> >>> So this is Qemu TCG mode right ? I will try some test and update later.
> >>>
> >>
> >> I am able to reproduce this with
> >>
> >> qemu-system-ppc64 -kernel vmlinux -machine
> >> type=pseries,usb=off -smp 1 -m 1G  -vga none -nographic-device
> >> usb-ehci -device usb-kbd -device usb-mouse
> >>
> >> Looks like enabling usb device is the issue.
> >>
> >
> > Hmm Qemu  does
> >
> > /* Looks like an IO address */
> > /* FIXME: What WIMG combinations could be sensible for IO?
> >  * For now we allow WIMG=010x, but are there others? */
> > /* FIXME: Should we check against registered IO addresses? */
> > if ((ptel & (HPTE64_R_W | HPTE64_R_I | HPTE64_R_M)) != HPTE64_R_I) {
> > return H_PARAMETER;
> > }
> >
> 
> 
> Can you try this patch ?
> 
> Ben,
> 
> The pHyp part of the comment was added by you in 
> 3c726f8dee6f55e96475574e9f645327e461884c ([PATCH] ppc64: support 64k
> page) really an old commit. I also remember we having the discussion and
> concluding that it should be safe to assume that we can always enable
> memory coherence. Should we do the below patch of get Qemu fixed up ?
> like below
> 
> -if ((ptel & (HPTE64_R_W | HPTE64_R_I | HPTE64_R_M)) != HPTE64_R_I) {
> +wimg_flags = (ptel & (HPTE64_R_W | HPTE64_R_I | HPTE64_R_M));
> +
> +if (wimg_flags != HPTE64_R_I && wimg_flags != (HPTE64_R_I | 
> HPTE64_R_M)) {
>  return H_PARAMETER;
>  }
> 
> 
> commit 58f32706f64ee8fe21bbd7d6c211720cedec1292
> Author: Aneesh Kumar K.V <aneesh.ku...@linux.vnet.ibm.com>
> Date:   Thu Jun 16 19:43:39 2016 +0530
> 
> powerpc/mm/hash: Don't add memory coherence if cache inhibited is set
> 
> H_ENTER hcall handling in qemu had assumptions that a cache inhibited
> hpte entry won't have memory conference set. Also older kernel

coherence ?^^

> mentioned that some version of pHyp required this (look at the comment
> removed by the commit mentioned below).
> 
> Fixes: commit 30bda41aba4e("powerpc/mm: Drop WIMG in favour of new 
> constant")
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@linux.vnet.ibm.com>
> 
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index fcf676f7f522..89c2791bc510 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -201,9 +201,8 @@ unsigned long htab_convert_pte_flags(unsigned long 
> pteflags)
>   /*
>* We can't allow hardware to update hpte bits. Hence always
>* set 'R' bit and set 'C' if it is a write fault
> -  * Memory coherence is always enabled
>*/
> - rflags |=  HPTE_R_R | HPTE_R_M;
> + rflags |=  HPTE_R_R;
>  
>   if (pteflags & _PAGE_DIRTY)
>   rflags |= HPTE_R_C;
> @@ -216,7 +215,12 @@ unsigned long htab_convert_pte_flags(unsigned long 
> pteflags)
>   if ((pteflags & _PAGE_CACHE_CTL ) == _PAGE_NON_IDEMPOTENT)
>   rflags |= (HPTE_R_I | HPTE_R_G);
>   if ((pteflags & _PAGE_CACHE_CTL) == _PAGE_SAO)
> - rflags |= (HPTE_R_I | HPTE_R_W);
> + rflags |= (HPTE_R_W | HPTE_R_I | HPTE_R_M);
> + /*
> +  * Add memory coherence if cache inhibited is not set
> +  */
> + if (!(rflags & HPTE_R_I))
> + rflags |= HPTE_R_M;
>  
>   return rflags;
>  }
>

In any case, the VM boots again, thank you!

--D
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: kernel bug in "Drop WIMG in favour of new constants"?

[Darrick J. Wong]

On Thu, Jun 16, 2016 at 04:16:00PM +0530, Aneesh Kumar K.V wrote:
> "Aneesh Kumar K.V" <aneesh.ku...@linux.vnet.ibm.com> writes:
> 
> > "Darrick J. Wong" <darrick.w...@oracle.com> writes:
> >
> >> On Thu, Jun 16, 2016 at 03:23:47PM +1000, Michael Ellerman wrote:
> >>> On Wed, 2016-06-15 at 21:33 -0700, Darrick J. Wong wrote:
> >>> 
> >>> > Hi Aneesh,
> >>> > 
> >>> > I noticed when trying out 4.7-rc3 on qemu-2.5 that the kernel no longer
> >>> > boots.  4.6 booted just fine, so I bisected the kernel to the commit
> >>> > 30bda41aba4efb2370c97e2cbe7385de93ccc372, which is "powerpc/mm: Drop 
> >>> > WIMG in
> >>> > favour of new constants".  The changelog suggests that the KVM changes 
> >>> > need
> >>> > closer review, and here's an actual crash:
> >>> > 
> >>> > (I can send libvirt's machine xml, .config, and full dmesg if that 
> >>> > helps.)
> >>> 
> >>> Yes please.
> >>> 
> >>> I'm successfully booting 4.7-rc's on qemu (2.5.0 (Debian 
> >>> 1:2.5+dfsg-5ubuntu10.1)).
> >>
> >> Ok, see attached.  I also sent along the dpkg --status output for qemu
> >> and qemu-slof; looks like we're running the same Ubuntu packages...
> >>
> >> ...my host kernel is 4.6.0 on x64.
> >
> > So this is Qemu TCG mode right ? I will try some test and update later.
> >
> 
> I am able to reproduce this with
> 
> qemu-system-ppc64 -kernel vmlinux -machine
> type=pseries,usb=off -smp 1 -m 1G  -vga none -nographic-device
> usb-ehci -device usb-kbd -device usb-mouse
> 
> Looks like enabling usb device is the issue.

 Unfortunately, libvirt cleverly reinstalls the usb configuration
if I delete all the usb lines from the xml file.  I don't need USB, I'm
merely the stuckee. ;)

--D

> 
> -aneesh
> 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: kernel bug in "Drop WIMG in favour of new constants"?

[Darrick J. Wong]

On Thu, Jun 16, 2016 at 03:23:47PM +1000, Michael Ellerman wrote:
> On Wed, 2016-06-15 at 21:33 -0700, Darrick J. Wong wrote:
> 
> > Hi Aneesh,
> > 
> > I noticed when trying out 4.7-rc3 on qemu-2.5 that the kernel no longer
> > boots.  4.6 booted just fine, so I bisected the kernel to the commit
> > 30bda41aba4efb2370c97e2cbe7385de93ccc372, which is "powerpc/mm: Drop WIMG in
> > favour of new constants".  The changelog suggests that the KVM changes need
> > closer review, and here's an actual crash:
> > 
> > (I can send libvirt's machine xml, .config, and full dmesg if that helps.)
> 
> Yes please.
> 
> I'm successfully booting 4.7-rc's on qemu (2.5.0 (Debian 
> 1:2.5+dfsg-5ubuntu10.1)).

Ok, see attached.  I also sent along the dpkg --status output for qemu
and qemu-slof; looks like we're running the same Ubuntu packages...

...my host kernel is 4.6.0 on x64.

--D
Connected to domain mtrp0
Escape character is ^]
Populating /vdevice methods
Populating /vdevice/vty@3000
Populating /vdevice/nvram@7100
Populating /pci@8002000
 00 4000 (D) : 1af4 1005unknown-legacy-device*
 00 3800 (D) : 1af4 1000virtio [ net ]
 00 3000 (D) : 1af4 1000virtio [ net ]
 00 2800 (D) : 1af4 1002unknown-legacy-device*
 00 2000 (D) : 1af4 1003communication-controller*
 00 1800 (D) : 1af4 1004virtio [ scsi ]
Populating /pci@8002000/scsi@3
   SCSI: Looking for devices
  101 DISK : "QEMU QEMU HARDDISK2.5+"
  100 DISK : "QEMU QEMU HARDDISK2.5+"
 00 1000 (D) : 1033 0194serial bus [ usb-xhci ]
 00 0800 (D) : 1234 qemu vga
Installing QEMU fb



Scanning USB 
  XHCI: Initializing
USB Keyboard 
USB mouse 
USB HUB 
No console specified using screen & keyboard
Detected RAM kernel at 40 (103dad8 bytes)  
  Welcome to Open Firmware

  Copyright (c) 2004, 2011 IBM Corporation All rights reserved.
  This program and the accompanying materials are made available
  under the terms of the BSD License available at
  http://www.opensource.org/licenses/bsd-license.php

Booting from memory...
[0.00] bootconsole [udbg0] enabled
 -> early_setup(), dt_ptr: 0x1942000
[0.00] Allocated 4096 bytes for 1 pacas at c000
[0.00] pSeries detected, looking for LPAR capability...
[0.00]  -> fw_vec5_feature_init()
[0.00]  <- fw_vec5_feature_init()
[0.00]  -> fw_hypertas_feature_init()
[0.00]  <- fw_hypertas_feature_init()
[0.00] Machine is LPAR !
[0.00] Using pSeries machine description
Found, Initializing memory management...
[0.00] Page sizes from device-tree:
[0.00] base_shift=12: shift=12, sllp=0x, avpnm=0x, 
tlbiel=1, penc=0
[0.00] base_shift=24: shift=24, sllp=0x0100, avpnm=0x0001, 
tlbiel=0, penc=0
[0.00] Page orders: linear mapping = 24, virtual = 12, io = 12, vmemmap 
= 24
[0.00] Using 1TB segments
 <- early_setup()
 -> setup_system()
 -> initialize_cache_info()
 <- initialize_cache_info()
[0.00] Found initrd at 0xc145:0xc18d8cd3
[0.00]  -> pSeries_init_early()
[0.00]  -> fw_cmo_feature_init()
[0.00] CMO not available
[0.00]  <- fw_cmo_feature_init()
[0.00]  <- pSeries_init_early()
[0.00] Starting Linux ppc64 #11 PREEMPT Wed Jun 15 18:03:32 PDT 2016
[0.00] -
[0.00] ppc64_pft_size= 0x18
[0.00] phys_mem_size = 0x8000
[0.00] cpu_features  = 0x0b7e7a6418500049
[0.00]   possible= 0x3f7f18500649
[0.00]   always  = 0x18100040
[0.00] cpu_user_features = 0xdc0065c2 0x2000
[0.00] mmu_features  = 0x7c01
[0.00] firmware_features = 0x0001405a440b
[0.00] htab_hash_mask= 0x1
[0.00] -
 <- setup_system()
[0.00] Linux version 4.7.0-rc3-pcsum (djw...@alder.djwong.org) (gcc 
version 5.3.1 20160413 (Ubuntu 5.3.1-14ubuntu2) ) #11 PREEMPT Wed Jun 15 
18:03:32 PDT 2016
[0.00] PCI host bridge /pci@8002000  ranges:
[0.00]   IO 0x01008000..0x01008000 -> 0x
[0.00]  MEM 0x0100a000..0x01101fff -> 
0x8000 
[0.00] PPC64 nvram contains 131072 bytes
[0.00] Relocation on exceptions not supported
[0.00] Top of RAM: 0x8000, Total RAM: 0x8000
[0.00] Memory hole size: 0MB
[0.00] Zone ranges:
[0.00]   DMA  [mem 0x0

kernel bug in "Drop WIMG in favour of new constants"?

[Darrick J. Wong]

Hi Aneesh,

I noticed when trying out 4.7-rc3 on qemu-2.5 that the kernel no longer
boots.  4.6 booted just fine, so I bisected the kernel to the commit
30bda41aba4efb2370c97e2cbe7385de93ccc372, which is "powerpc/mm: Drop WIMG in
favour of new constants".  The changelog suggests that the KVM changes need
closer review, and here's an actual crash:

(I can send libvirt's machine xml, .config, and full dmesg if that helps.)

--Darrick

dmesg from the crash looks like:

[0.474264] mm: Hashing failure ! EA=0xd8008010 
access=0x800c current=swapper
[0.474777] trap=0x300 vsid=0x13d349c ssize=1 base psize=0 psize 0 
pte=0xc10121ae
[0.475102] mm: Hashing failure ! EA=0xd8008010 
access=0x800c current=swapper
[0.475363] trap=0x300 vsid=0x13d349c ssize=1 base psize=0 psize 0 
pte=0xc10121ae
[0.475805] Unable to handle kernel paging request for data at address 
0xd8008010
[0.476060] Faulting instruction address: 0xc053b4bc
[0.476415] Oops: Kernel access of bad area, sig: 7 [#1]
[0.476604] PREEMPT pSeries
[0.476904] Modules linked in:
[0.477192] CPU: 0 PID: 1 Comm: swapper Not tainted 4.7.0-rc3-pcsum #11
[0.477523] task: c0007f95 ti: c0007f94c000 task.ti: 
c0007f94c000
[0.477757] NIP: c053b4bc LR: c053b4a8 CTR: 
[0.477985] REGS: c0007f94f740 TRAP: 0300   Not tainted  
(4.7.0-rc3-pcsum)
[0.478205] MSR: 82009032   CR: 84000242  
XER: 
[0.478640] CFAR: c00083b4 DAR: d8008010 DSISR: 4000 
SOFTE: 1 
GPR00: c053b4a8 c0007f94f9c0 c0e24000 d8008010 
GPR04: d80080003000 c0007aecf018 4000 7f9c8000 
GPR08: 7aecf000 c101200031ae 01ff 0300 
GPR12: 84000242 c000 c000b2a0  
GPR16:     
GPR20:    c094a3b8 
GPR24: c0d952a8 c099a098 c102c250 c0e51088 
GPR28:  d8008000 4000 c0007ae1b000 
[0.481144] NIP [c053b4bc] .quirk_usb_early_handoff+0x48c/0xd00
[0.481365] LR [c053b4a8] .quirk_usb_early_handoff+0x478/0xd00
[0.481602] Call Trace:
[0.481763] [c0007f94f9c0] [c053b4a8] 
.quirk_usb_early_handoff+0x478/0xd00 (unreliable)
[0.482119] [c0007f94fab0] [c04312ac] .pci_do_fixups+0xdc/0x140
[0.482368] [c0007f94fb60] [c098427c] 
.pci_apply_final_quirks+0xb0/0x194
[0.482621] [c0007f94fc10] [c000aa48] .do_one_initcall+0x68/0x1e0
[0.482853] [c0007f94fcf0] [c0957278] 
.kernel_init_freeable+0x238/0x320
[0.483108] [c0007f94fdb0] [c000b2c4] .kernel_init+0x24/0x160
[0.483351] [c0007f94fe30] [c0009230] 
.ret_from_kernel_thread+0x58/0xa8
[0.483618] Instruction dump:
[0.483808] 7d3e07b4 4bfff9e1 2fa3 41befc8c e87f0320 7fc4f378 4bb093fd 
6000 
[0.484163] 7c7d1b79 4182fc74 387d0010 7c0004ac <7d201c2c> 0c09 4c00012c 
2f89 
[0.485685] ---[ end trace 38f2f4e017e75b42 ]---
[0.509029] 
[1.512619] Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x0007

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: ADT746X: logical-bitwise confusion in set_max_duty_at_crit()

[Darrick J. Wong]

On Mon, Mar 10, 2008 at 10:59:43AM +0100, Roel Kluin wrote:

  The  0xff here is bogus anyway; temp is only ever used as an u8,
  so just declare it as that, or do proper overflow/underflow checking
  on it.  The patch will need testing on hardware too, since it changes
  behaviour (it should be a bugfix, but who knows).
 
 Maybe someone can test this?

I did.  No regressions observed and it fixes that bug as well.  Sorry I
didn't catch it earlier... :/

Acked-by: Darrick J. Wong [EMAIL PROTECTED]

--D
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev