Re: [PATCH 2/3] RDMA/hfi1, rdmavt: open-code rvt_set_ibdev_name()

[Arnd Bergmann]

On Mon, Mar 23, 2026, at 09:08, Leon Romanovsky wrote:
> On Fri, Mar 20, 2026 at 04:53:04PM +0100, Arnd Bergmann wrote:
>> On Fri, Mar 20, 2026, at 16:12, Arnd Bergmann wrote:
>> 
>> > +	 */
>> > +	ibdev = &dd->verbs_dev.rdi.ibdev;
>> > +	dev_set_name(&ibdev->dev, "%s_%d", class_name(), dd->unit);
>> > +	strscpy(&ibdev->name, dev_name(&ibdev->dev), IB_DEVICE_NAME_MAX);
>> > +
>> 
>> I messed this up during a rebase, that should have been 
>> 
>>        strscpy(ibdev->name, dev_name(&ibdev->dev), IB_DEVICE_NAME_MAX);
>> 
>> (without the extra &). I'll wait for comments before resending.
>
> The hfi1 driver is scheduled for removal. Dennis has already posted the
> hfi2 driver, which serves as its replacement.

Ok, that does sound like a sensible decision, and I'll just drop
patches 1 and 3 then, which are just cleanups.

The cover letter at [1] suggests that the two drivers will still
coexist for a bit though, so I think we'd still want patch 2/3
in order to get a clean 'allmodconfig' build when the 
-Wmissing-format-attribute is enabled by defaultt. I have a couple
of patches in flight.

I took a quick look at the hfi2 driver, and noticed a few things
that that may be worth addressing before it gets merged, mostly
stuff copied from hfi1:

- A few global functions with questionable namespacing:
  user_event_ack, ctxt_reset, iowait_init, register_pinning_interface,
  sc_{alloc,free,enable,disable}, pio_copy, acquire_hw_mutex,
  load_firmware, cap_mask.
  It would make sense to prefix all global identifiers with 'hfi2_',
  both out of principle, and to allow building hfi1 and hfi2 into
  an allyesconfig kernel without link failures.

- The use of INFINIBAND_RDMAVT seems unnecessary: right now
  this is only used by hfi1, now shared with hfi2 but later to
  be exclusive to the latter. Since it is unlikely to ever
  be used by another driver again, this may be a good time
  to drop the abstraction again and integrate it all into
  hfi2, with the old version getting dropped along with hfi1.

- The pio_copy() function is particularly slow since it uses
  the full-barrier version of writeq() in a tight loop,
  this should probably use __iowrite64_copy() etc to make it
  work better on arm64 and other architectures

- The use of bitfields in drivers/infiniband/hw/hfi2/cport.h
  makes the structures nonportable especially for big-endian
  targets, if those describe device interfaces. Similarly
  the use of __packed attributes in the same file seems
  arbitrary and inconsistent, to the point where it
  is likely to cause more harm than it can help. E.g.
  in

+struct ib_cc_table_entry_shadow {
+	u16 entry; /* shift:2, multiplier:14 */
+};
+
>
+struct ib_cc_table_attr_shadow {
+	u16 ccti_limit; /* max CCTI for cc table */
+	struct ib_cc_table_entry_shadow ccti_entries[IB_CCT_ENTRIES];
+} __packed;

  the outer structure is unaligned while the inner one requires
  alignment.


     Arnd

[1] https://lore.kernel.org/all/177325138778.57064.8330693913074464417.stgit@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx/