* Re: [PATCH v1 3/5] treewide: use get_random_u32() when possible [not found] ` <CAHmME9r_vNRFFjUvqx8QkBddg_kQU=FMgpk9TqOVZdvX6zXHNg@mail.gmail.com> @ 2022-10-06 13:15 ` Jason Gunthorpe 2022-10-06 13:20 ` Andy Shevchenko 1 sibling, 0 replies; 5+ messages in thread From: Jason Gunthorpe @ 2022-10-06 13:15 UTC (permalink / raw) To: Jason A. Donenfeld Cc: linux-kernel, brcm80211-dev-list.pdl, cake, ceph-devel, coreteam, dccp, dev, dmaengine, drbd-dev, dri-devel, kasan-dev, linux-actions, linux-arm-kernel, linux-block, linux-crypto, linux-doc, linux-ext4, linux-f2fs-devel, linux-fbdev, linux-fsdevel, linux-hams, linux-media, linux-mm, linux-mmc, linux-mtd, linux-nfs, linux-nvme, linux-raid, linux-rdma, linux-scsi, linux-sctp, linux-stm32, linux-usb, linux-wireless, linux-xfs, linuxppc-dev, lvs-devel, netdev, netfilter-devel, rds-devel, SHA-cyfmac-dev-list, target-devel, tipc-discussion On Thu, Oct 06, 2022 at 07:05:48AM -0600, Jason A. Donenfeld wrote: > > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c > > > index fd9d7f2c4d64..a605cf66b83e 100644 > > > --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c > > > +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c > > > @@ -465,7 +465,7 @@ static int ipoib_cm_req_handler(struct ib_cm_id *cm_id, > > > goto err_qp; > > > } > > > > > > - psn = prandom_u32() & 0xffffff; > > > + psn = get_random_u32() & 0xffffff; > > > > prandom_max(0xffffff + 1) > > That'd work, but again it's not more clear. Authors here are going for > a 24-bit number, and masking seems like a clear way to express that. vs just asking directly for a 24 bit number? Jason _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 3/5] treewide: use get_random_u32() when possible [not found] ` <CAHmME9r_vNRFFjUvqx8QkBddg_kQU=FMgpk9TqOVZdvX6zXHNg@mail.gmail.com> 2022-10-06 13:15 ` [PATCH v1 3/5] treewide: use get_random_u32() when possible Jason Gunthorpe @ 2022-10-06 13:20 ` Andy Shevchenko 1 sibling, 0 replies; 5+ messages in thread From: Andy Shevchenko @ 2022-10-06 13:20 UTC (permalink / raw) To: Jason A. Donenfeld Cc: Jason Gunthorpe, linux-kernel, brcm80211-dev-list.pdl, cake, ceph-devel, coreteam, dccp, dev, dmaengine, drbd-dev, dri-devel, kasan-dev, linux-actions, linux-arm-kernel, linux-block, linux-crypto, linux-doc, linux-ext4, linux-f2fs-devel, linux-fbdev, linux-fsdevel, linux-hams, linux-media, linux-mm, linux-mmc, linux-mtd, linux-nfs, linux-nvme, linux-raid, linux-rdma, linux-scsi, linux-sctp, linux-stm32, linux-usb, linux-wireless, linux-xfs, linuxppc-dev, lvs-devel, netdev, netfilter-devel, rds-devel, SHA-cyfmac-dev-list, target-devel, tipc-discussion On Thu, Oct 06, 2022 at 07:05:48AM -0600, Jason A. Donenfeld wrote: > On Thu, Oct 6, 2022 at 6:47 AM Jason Gunthorpe <jgg@ziepe.ca> wrote: > > On Wed, Oct 05, 2022 at 11:48:42PM +0200, Jason A. Donenfeld wrote: ... > > > - u32 isn = (prandom_u32() & ~7UL) - 1; > > > + u32 isn = (get_random_u32() & ~7UL) - 1; > > > > Maybe this wants to be written as > > > > (prandom_max(U32_MAX >> 7) << 7) | 7 > > ? > > Holy smokes. Yea I guess maybe? It doesn't exactly gain anything or > make the code clearer though, and is a little bit more magical than > I'd like on a first pass. Shouldn't the two first 7s to be 3s? ... > > > - psn = prandom_u32() & 0xffffff; > > > + psn = get_random_u32() & 0xffffff; > > > > prandom_max(0xffffff + 1) > > That'd work, but again it's not more clear. Authors here are going for > a 24-bit number, and masking seems like a clear way to express that. We have some 24-bit APIs (and 48-bit) already in kernel, why not to have get_random_u24() ? -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 3/5] treewide: use get_random_u32() when possible [not found] ` <20221005214844.2699-4-Jason@zx2c4.com> [not found] ` <Yz7OdfKZeGkpZSKb@ziepe.ca> @ 2022-10-12 19:16 ` Joe Perches 2022-10-12 21:29 ` David Laight 1 sibling, 1 reply; 5+ messages in thread From: Joe Perches @ 2022-10-12 19:16 UTC (permalink / raw) To: Jason A. Donenfeld, linux-kernel Cc: brcm80211-dev-list.pdl, cake, ceph-devel, coreteam, dccp, dev, dmaengine, drbd-dev, dri-devel, kasan-dev, linux-actions, linux-arm-kernel, linux-block, linux-crypto, linux-doc, linux-ext4, linux-f2fs-devel, linux-fbdev, linux-fsdevel, linux-hams, linux-media, linux-mm, linux-mmc, linux-mtd, linux-nfs, linux-nvme, linux-raid, linux-rdma, linux-scsi, linux-sctp, linux-stm32, linux-usb, linux-wireless, linux-xfs, linuxppc-dev, lvs-devel, netdev, netfilter-devel, rds-devel, SHA-cyfmac-dev-list, target-devel, tipc-discussion On Wed, 2022-10-05 at 23:48 +0200, 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. [] > diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c [] > @@ -734,7 +734,7 @@ static int send_connect(struct c4iw_ep *ep) > &ep->com.remote_addr; > int ret; > enum chip_type adapter_type = ep->com.dev->rdev.lldi.adapter_type; > - u32 isn = (prandom_u32() & ~7UL) - 1; > + u32 isn = (get_random_u32() & ~7UL) - 1; trivia: There are somewhat odd size mismatches here. I had to think a tiny bit if random() returned a value from 0 to 7 and was promoted to a 64 bit value then truncated to 32 bit. Perhaps these would be clearer as ~7U and not ~7UL > struct net_device *netdev; > u64 params; > > @@ -2469,7 +2469,7 @@ static int accept_cr(struct c4iw_ep *ep, struct sk_buff *skb, > } > > if (!is_t4(adapter_type)) { > - u32 isn = (prandom_u32() & ~7UL) - 1; > + u32 isn = (get_random_u32() & ~7UL) - 1; etc... drivers/infiniband/hw/cxgb4/cm.c: u32 isn = (prandom_u32() & ~7UL) - 1; drivers/infiniband/hw/cxgb4/cm.c: u32 isn = (prandom_u32() & ~7UL) - 1; drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c: rpl5->iss = cpu_to_be32((prandom_u32() & ~7UL) - 1); drivers/scsi/cxgbi/cxgb4i/cxgb4i.c: u32 isn = (prandom_u32() & ~7UL) - 1; drivers/scsi/cxgbi/cxgb4i/cxgb4i.c: u32 isn = (prandom_u32() & ~7UL) - 1; drivers/target/iscsi/cxgbit/cxgbit_cm.c: rpl5->iss = cpu_to_be32((prandom_u32() & ~7UL) - 1); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v1 3/5] treewide: use get_random_u32() when possible 2022-10-12 19:16 ` Joe Perches @ 2022-10-12 21:29 ` David Laight 2022-10-13 1:37 ` Joe Perches 0 siblings, 1 reply; 5+ messages in thread From: David Laight @ 2022-10-12 21:29 UTC (permalink / raw) To: 'Joe Perches', Jason A. Donenfeld, linux-kernel Cc: linux-fbdev, linux-doc, linux-wireless, dri-devel, linux-mm, linux-sctp, target-devel, linux-mtd, linux-stm32, drbd-dev, dev, rds-devel, linux-scsi, dccp, linux-rdma, kasan-dev, lvs-devel, SHA-cyfmac-dev-list, coreteam, tipc-discussion, linux-ext4, linux-media, linux-actions, linux-nfs, linux-block, dmaengine, linux-nvme, linux-hams, ceph-devel, linux-arm-kernel, cake, brcm80211-dev-list.pdl, linux-raid, netdev, linux-usb, linux-mmc, linux-f2fs-devel, linux-xfs, netfilter-devel, linux-crypto, linux-fsdevel, linuxppc-dev From: Joe Perches > Sent: 12 October 2022 20:17 > > On Wed, 2022-10-05 at 23:48 +0200, 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. > [] > > diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c > [] > > @@ -734,7 +734,7 @@ static int send_connect(struct c4iw_ep *ep) > > &ep->com.remote_addr; > > int ret; > > enum chip_type adapter_type = ep->com.dev->rdev.lldi.adapter_type; > > - u32 isn = (prandom_u32() & ~7UL) - 1; > > + u32 isn = (get_random_u32() & ~7UL) - 1; > > trivia: > > There are somewhat odd size mismatches here. > > I had to think a tiny bit if random() returned a value from 0 to 7 > and was promoted to a 64 bit value then truncated to 32 bit. > > Perhaps these would be clearer as ~7U and not ~7UL That makes no difference - the compiler will generate the same code. The real question is WTF is the code doing? The '& ~7u' clears the bottom 3 bits. The '- 1' then sets the bottom 3 bits and decrements the (random) high bits. So is the same as get_random_u32() | 7. But I bet the coder had something else in mind. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 3/5] treewide: use get_random_u32() when possible 2022-10-12 21:29 ` David Laight @ 2022-10-13 1:37 ` Joe Perches 0 siblings, 0 replies; 5+ messages in thread From: Joe Perches @ 2022-10-13 1:37 UTC (permalink / raw) To: David Laight, Jason A. Donenfeld, linux-kernel Cc: linux-fbdev, linux-doc, linux-wireless, dri-devel, linux-mm, linux-sctp, target-devel, linux-mtd, linux-stm32, drbd-dev, dev, rds-devel, linux-scsi, dccp, linux-rdma, kasan-dev, lvs-devel, SHA-cyfmac-dev-list, coreteam, tipc-discussion, linux-ext4, linux-media, linux-actions, linux-nfs, linux-block, dmaengine, linux-nvme, linux-hams, ceph-devel, linux-arm-kernel, cake, brcm80211-dev-list.pdl, linux-raid, netdev, linux-usb, linux-mmc, linux-f2fs-devel, linux-xfs, netfilter-devel, linux-crypto, linux-fsdevel, linuxppc-dev On Wed, 2022-10-12 at 21:29 +0000, David Laight wrote: > From: Joe Perches > > Sent: 12 October 2022 20:17 > > > > On Wed, 2022-10-05 at 23:48 +0200, 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. > > [] > > > diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c > > [] > > > @@ -734,7 +734,7 @@ static int send_connect(struct c4iw_ep *ep) > > > &ep->com.remote_addr; > > > int ret; > > > enum chip_type adapter_type = ep->com.dev->rdev.lldi.adapter_type; > > > - u32 isn = (prandom_u32() & ~7UL) - 1; > > > + u32 isn = (get_random_u32() & ~7UL) - 1; > > > > trivia: > > > > There are somewhat odd size mismatches here. > > > > I had to think a tiny bit if random() returned a value from 0 to 7 > > and was promoted to a 64 bit value then truncated to 32 bit. > > > > Perhaps these would be clearer as ~7U and not ~7UL > > That makes no difference - the compiler will generate the same code. True, more or less. It's more a question for the reader. > The real question is WTF is the code doing? True. > The '& ~7u' clears the bottom 3 bits. > The '- 1' then sets the bottom 3 bits and decrements the > (random) high bits. Right. > So is the same as get_random_u32() | 7. True, it's effectively the same as the upper 29 bits are random anyway and the bottom 3 bits are always set. > But I bet the coder had something else in mind. Likely. And it was also likely copy/pasted a few times. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-13 1:38 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20221005214844.2699-1-Jason@zx2c4.com> [not found] ` <20221005214844.2699-4-Jason@zx2c4.com> [not found] ` <Yz7OdfKZeGkpZSKb@ziepe.ca> [not found] ` <CAHmME9r_vNRFFjUvqx8QkBddg_kQU=FMgpk9TqOVZdvX6zXHNg@mail.gmail.com> 2022-10-06 13:15 ` [PATCH v1 3/5] treewide: use get_random_u32() when possible Jason Gunthorpe 2022-10-06 13:20 ` Andy Shevchenko 2022-10-12 19:16 ` Joe Perches 2022-10-12 21:29 ` David Laight 2022-10-13 1:37 ` Joe Perches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).