linux-hyperv.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Naman Jain <namjain@linux.microsoft.com>
To: "K . Y . Srinivasan" <kys@microsoft.com>,
	Haiyang Zhang <haiyangz@microsoft.com>,
	Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Stephen Hemminger <stephen@networkplumber.org>
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@kernel.org, Saurabh Sengar <ssengar@linux.microsoft.com>,
	Michael Kelley <mhklinux@outlook.com>,
	Naman Jain <namjain@linux.microsoft.com>
Subject: [PATCH v7 0/2] uio_hv_generic: Fix ring buffer sysfs creation path
Date: Fri,  2 May 2025 13:18:09 +0530	[thread overview]
Message-ID: <20250502074811.2022-1-namjain@linux.microsoft.com> (raw)

Hi,
This patch series aims to address the sysfs creation issue for the ring
buffer by reorganizing the code. Additionally, it updates the ring sysfs
size to accurately reflect the actual ring buffer size, rather than a
fixed static value.

PFB change logs:

Changes since v6:
https://lore.kernel.org/all/20250424053524.1631-1-namjain@linux.microsoft.com/
* Rebased to 6.15-rc4 tip where [1] is not present to fix compilation
  issue on 6.15-rc4 and to enable porting of the fix to previous kernel
  versions where patch [1] is not present. (addressed Greg's comments)
* Note: continued to use bin_attrs and not bin_attrs_new so that the fix
  can be ported to older kernels where bin_attrs_new was not introduced.

[1]: commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs")

Changes since v5:
https://lore.kernel.org/all/20250415164452.170239-1-namjain@linux.microsoft.com/
* Added Reviewed-By tags from Dexuan. Also, addressed minor comments in
  commit msg of both patches.
* Missed to remove check for "primary_channel->device_obj->channels_kset" in
  hv_create_ring_sysfs in earlier patch, as suggested by Michael. Did it
  now. 
* Changed type for declaring bin_attrs due to changes introduced by
  commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") which
  merged recently. Did not use bin_attrs_new since another change is in
  the queue to change usage of bin_attrs_new to bin_attrs
  (sysfs: finalize the constification of 'struct bin_attribute').

Changes since v4:
https://lore.kernel.org/all/20250410060847.82407-1-namjain@linux.microsoft.com/
* Added Reviewed-By and Tested-By tags from Michael.
* Fixed syntax by removing extra space (addressed Greg's comment)
* Rebased to latest linux-next tip

Changes since v3:
https://lore.kernel.org/all/20250328052745.1417-1-namjain@linux.microsoft.com/
* Addressed Michael's comments regarding handling of return value of
sysfs_update_group in uio_hv_generic.

Changes since v2:
https://lore.kernel.org/all/20250318061558.3294-1-namjain@linux.microsoft.com/
Addressed Greg's comments:
* Split the original patch into two.
* Updated the commit message to explain the problem scenario.
* Added comments for new APIs in the kerneldoc format.
* Highlighted potential race conditions and explained why sysfs should not be created in the
  driver probe.

* Made minor changes to how the sysfs_update_group return value is handled.

Changes since v1:
https://lore.kernel.org/all/20250225052001.2225-1-namjain@linux.microsoft.com/
* Fixed race condition in setting channel->mmap_ring_buffer by
  introducing a new variable for visibility of sysfs (addressed Greg's
  comments)
* Used binary attribute fields instead of regular ones for initializing attribute_group.
* Make size of ring sysfs dynamic based on actual ring buffer's size.
* Preferred to keep mmap function in uio_hv_generic to give more control over ring's
  mmap functionality, since this is specific to uio_hv_generic driver.
* Remove spurious warning during sysfs creation in uio_hv_generic probe.
* Added comments in a couple of places.

Changes since RFC patch:
https://lore.kernel.org/all/20250214064351.8994-1-namjain@linux.microsoft.com/
* Different approach to solve the problem is proposed (credits to
  Michael Kelley).
* Core logic for sysfs creation moved out of uio_hv_generic, to VMBus
  drivers where rest of the sysfs attributes for a VMBus channel
  are defined. (addressed Greg's comments)
* Used attribute groups instead of sysfs_create* functions, and bundled
  ring attribute with other attributes for the channel sysfs.  

Error logs:

[   35.574120] ------------[ cut here ]------------
[   35.574122] WARNING: CPU: 0 PID: 10 at fs/sysfs/file.c:591 sysfs_create_bin_file+0x81/0x90
[   35.574168] Workqueue: hv_pri_chan vmbus_add_channel_work
[   35.574172] RIP: 0010:sysfs_create_bin_file+0x81/0x90
[   35.574197] Call Trace:
[   35.574199]  <TASK>
[   35.574200]  ? show_regs+0x69/0x80
[   35.574217]  ? __warn+0x8d/0x130
[   35.574220]  ? sysfs_create_bin_file+0x81/0x90
[   35.574222]  ? report_bug+0x182/0x190
[   35.574225]  ? handle_bug+0x5b/0x90
[   35.574244]  ? exc_invalid_op+0x19/0x70
[   35.574247]  ? asm_exc_invalid_op+0x1b/0x20
[   35.574252]  ? sysfs_create_bin_file+0x81/0x90
[   35.574255]  hv_uio_probe+0x1e7/0x410 [uio_hv_generic]
[   35.574271]  vmbus_probe+0x3b/0x90
[   35.574275]  really_probe+0xf4/0x3b0
[   35.574279]  __driver_probe_device+0x8a/0x170
[   35.574282]  driver_probe_device+0x23/0xc0
[   35.574285]  __device_attach_driver+0xb5/0x140
[   35.574288]  ? __pfx___device_attach_driver+0x10/0x10
[   35.574291]  bus_for_each_drv+0x86/0xe0
[   35.574294]  __device_attach+0xc1/0x200
[   35.574297]  device_initial_probe+0x13/0x20
[   35.574315]  bus_probe_device+0x99/0xa0
[   35.574318]  device_add+0x647/0x870
[   35.574320]  ? hrtimer_init+0x28/0x70
[   35.574323]  device_register+0x1b/0x30
[   35.574326]  vmbus_device_register+0x83/0x130
[   35.574328]  vmbus_add_channel_work+0x135/0x1a0
[   35.574331]  process_one_work+0x177/0x340
[   35.574348]  worker_thread+0x2b2/0x3c0
[   35.574350]  kthread+0xe3/0x1f0
[   35.574353]  ? __pfx_worker_thread+0x10/0x10
[   35.574356]  ? __pfx_kthread+0x10/0x10


Naman Jain (2):
  uio_hv_generic: Fix sysfs creation path for ring buffer
  Drivers: hv: Make the sysfs node size for the ring buffer dynamic

 drivers/hv/hyperv_vmbus.h    |   6 ++
 drivers/hv/vmbus_drv.c       | 109 ++++++++++++++++++++++++++++++++++-
 drivers/uio/uio_hv_generic.c |  39 ++++++-------
 include/linux/hyperv.h       |   6 ++
 4 files changed, 137 insertions(+), 23 deletions(-)


base-commit: b4432656b36e5cc1d50a1f2dc15357543add530e
-- 
2.34.1


             reply	other threads:[~2025-05-02  7:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-02  7:48 Naman Jain [this message]
2025-05-02  7:48 ` [PATCH v7 1/2] uio_hv_generic: Fix sysfs creation path for ring buffer Naman Jain
2025-05-02  7:48 ` [PATCH v7 2/2] Drivers: hv: Make the sysfs node size for the ring buffer dynamic Naman Jain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250502074811.2022-1-namjain@linux.microsoft.com \
    --to=namjain@linux.microsoft.com \
    --cc=decui@microsoft.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=haiyangz@microsoft.com \
    --cc=kys@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhklinux@outlook.com \
    --cc=ssengar@linux.microsoft.com \
    --cc=stable@kernel.org \
    --cc=stephen@networkplumber.org \
    --cc=wei.liu@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).