| Age | Commit message (Collapse) | Author |
|---|
|
card removal
When removing a card instance, the underlying driver is replaced to handle
closing operations. Without proper memory reordering, other CPUs may
observe the driver before it is fully initialized.
Use a release/acquire pair to ensure that the driver's initialization is
visible before the driver is accessed for close operations.
Link: https://lore.kernel.org/r/20261017081720.186688-1-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
Some event structures have a trailing flexible array member of bytes to
store event-specific data. Use kzalloc_flex() to allocate the structure
together with storage for the flexible array member.
Link: https://lore.kernel.org/r/20260928125718.50974-4-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
Some event structures have a trailing flexible array member of quadlets
to store event-specific data. Use kzalloc_flex() to allocate the
structure together with storage for the flexible array member.
Link: https://lore.kernel.org/r/20260928125718.50974-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
All execution paths in the cdev layer now run in process context.
Use a mutex instead of a spinlock for client-level locking.
Link: https://lore.kernel.org/r/20260927230724.119305-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The transition state of an iso_resource_auto client resource has no
effect on the client, so the client-level lock is not suitable for
protecting it.
At present, this state is the only member that requires mutual
exclusion. Adding a separate lock for it would be excessive.
Change the type of the transition state to atomic_t and use an atomic
compare-and-swap operation to preserve consistency.
Link: https://lore.kernel.org/r/20260927230724.119305-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
Arrange the local variables to avoid a goto statement when copying events
to the userspace buffer.
Link: https://lore.kernel.org/r/20260927034343.28076-5-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
xchg() is useful for exchanging a pointer and returning the old value,
even when atomicity is not required.
Link: https://lore.kernel.org/r/20260927034343.28076-4-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
available
The in_shutdown member of client structure is set only when releasing
file descriptor, thus no need to wake up the client itself.
Link: https://lore.kernel.org/r/20260927034343.28076-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
Some accesses to fw_device members unnecessarily use the client-level
lock. Remove the locking from these accesses.
Link: https://lore.kernel.org/r/20260927034343.28076-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The client reference leaks when the pending work is cancelled because an
additional reference was taken when scheduling the work.
The reference held by the fw_iso_resource_auto object already ensures
that the client structure remains valid for the lifetime of the object.
Therefore, the additional reference taken when scheduling the work is
unnecessary.
Remove the additional reference.
Reported-by: Dingisoul <dingiso.kernel@gmail.com>
Link: https://sourceforge.net/p/linux1394/mailman/message/59317811/
Link: https://lore.kernel.org/r/20260923172328.277232-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The fw_iso_resource_auto object can outlive the operation that created
it since hte main operation for it is done in a work item. This work and
release paths access members of the client structure, therefore the
client structure must remain valid for the lifetime of an
fw_iso_resource_auto object.
Hold a reference to the client structure for the lifetime of the
fw_iso_resource_auto object.
Link: https://lore.kernel.org/r/20260923172328.277232-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The implementation of the client_resource structure now requires a release
callback.
Add an argument to add_client_resource() for the release callback.
Link: https://lore.kernel.org/r/20260920083659.191855-6-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The add_client_resource() function has an argument for GFP flags. The
argument was originally used to distinguish atomic calls. However, all
callers now use GFP_KERNEL.
Drop the unused argument and always allocate with GFP_KERNEL.
Link: https://lore.kernel.org/r/20260920083659.191855-5-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The callback for the address handler is now guaranteed to be invoked in
process context. In the cdev layer, GFP_ATOMIC is still used for memory
allocation in the address handler.
Use GFP_KERNEL instead.
Link: https://lore.kernel.org/r/20260920083659.191855-4-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
In the cdev layer, the list of phy packet receivers is currently
protected by a spinlock. This requires GFP_ATOMIC for memory allocation
when enumerating the list in fw_cdev_handle_phy_packet(). However, the
function is guaranteed to be invoked in process context, so GFP_KERNEL
can be used by switching from the spinlock to a mutex.
Link: https://lore.kernel.org/r/20260920083659.191855-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
Current implementation uses the spinlock in the card structure to
serialize accesses to the event list. The dequeue_event() function yields
the CPU while waiting for the condition to become true, but checks the
list without acquiring the spinlock. It acquires the spinlock when
operating the list, however it assumes that the list has at least one
entry.
This can cause problems when multiple threads execute read(2) system calls
concurrently, since the list can become empty after rescheduling but before
the list operation, depending on the scheduling order of the two threads.
Use a wait_event variant that reacquires the lock after being
rescheduled, before checking the condition.
Link: https://lore.kernel.org/r/20260920104745.212753-1-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The bus reset event structure exposed through the UAPI has a different
size depending on the target architecture. Extra care is required when
copying it to/from userspace.
Add a KUnit test for the structure to ensure that it has the expected
size and offsets.
Link: https://lore.kernel.org/r/20260920071501.77785-6-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
bus_reset_closure
There is a race condition involving the bus_reset_closure member of the
fw_cdev_event_bus_reset structure, which was addressed by commit
93b37905f70 ("firewire: cdev: prevent race between first get_info ioctl
and bus reset event queuing").
Add code comments and annotations to document it.
Link: https://lore.kernel.org/r/20260920071501.77785-5-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
A non-zero return value from copy_to_user() is handled by returning
-EFAULT. Using scoped_guard() simplifies this handling, since it
eliminates the need for a local variable to store the return value and
check it after the locking primitive is released.
Refactor the code to remove the local variable.
Link: https://lore.kernel.org/r/20260920071501.77785-4-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The scope of scoped_guard() corresponds to the code block, making it
easier to understand which code is protected by the locking primitive.
Link: https://lore.kernel.org/r/20260920071501.77785-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The core code uses a read memory barrier when accessing the generation
and node_id members of the fw_device structure. It assumes that the
latest value of the former is visible to a processor core when the latest
value of the latter is visible, since these members are updated in reverse
order with a write memory barrier.
However, the function that fills the fw_cdev_event_bus_reset structure
does not follow this approach. Use a read memory barrier when accessing
these members.
Link: https://lore.kernel.org/r/20260920071501.77785-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The todo member of iso_resource_auto structure represents the state of the
client resource and normally transitions in the following order:
ISO_RES_AUTO_ALLOC -> ISO_RES_AUTO_REALLOC -> ISO_RES_AUTO_DEALLOC
However, concurrent access from the work item and the file descriptor
release function can cause the state to transition backwards from
ISO_RES_AUTO_DEALLOC to ISO_RES_AUTO_REALLOC.
Prevent the back-transition by checking the current state before
updating it in the work item.
Fixes: fcabbf40fae5 ("firewire: core: move allocation/reallocation paths into specific branch after isoc resource management in cdev")
Link: https://lore.kernel.org/r/20260922132639.191593-1-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
There is no need to use deferrable type of work for iso_resource_once
management because the work is queued to run immediately.
Link: https://lore.kernel.org/r/20260520130840.629934-4-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The iso_resource_params structure has a member for channel mask, while
the name of field is easy to misinterpret.
Append _mask to the member name.
Link: https://lore.kernel.org/r/20260520130840.629934-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
resources management
The generation parameter is specific to the auto case of iso resources
management, while it is in the common parameter structure.
Move the generation member to the structure specific to auto case.
Link: https://lore.kernel.org/r/20260520130840.629934-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
after isoc resource management in cdev
After managing the actual isochronous resources, there is
post-processing logic to determine what type of event should be
notified. However, there is room for improvement.
This commit refactors the logic.
Link: https://lore.kernel.org/r/20260501135823.241940-5-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
management in cdev
After managing the actual isochronous resources, there is
post-processing logic to determine what type of event should be
notified. However, there is room for improvement.
This commit refactors the logic.
Link: https://lore.kernel.org/r/20260501135823.241940-4-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
management in cdev
The iso_resource_auto structure object has three states. The current
implementation of state evaluation before managing the actual isochronous
resources can be improved.
This commit refactors the evaluation logic using a switch statement.
Link: https://lore.kernel.org/r/20260501135823.241940-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
resource management in cdev
It is preferable for the critical section to be as small as possible.
Current implementation of iso_resource_auto_work() function uses a
spinlock to control concurrent access to members of fw_card, fw_device,
iso_resource_auto structures, however the locking duration could be
reduced.
This commit refactors to shorten that duration.
Link: https://lore.kernel.org/r/20260501135823.241940-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The init_iso_resource function is only called by
ioctl_allocate_iso_resource(), thus no need to be unique.
This commit unifies them with minor code refactoring.
Link: https://lore.kernel.org/r/20260429093449.160545-8-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The functions for iso_resource once operations are carefully split from
another type of operation.
This commit adds _auto suffix to functions for the another type so that
it is easily to distinguish them.
Link: https://lore.kernel.org/r/20260429093449.160545-7-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The helper functions for iso_resource allocation and work item still
include codes for once operation.
This commit refactors them to remove the old implementations.
Link: https://lore.kernel.org/r/20260429093449.160545-6-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
Unlike FW_CDEV_IOC_ALLOCATE_ISO_RESOURCE operation, the operations of
FW_CDEV_IOC_[DE]ALLOCATE_ISO_RESOURCE_ONCE require no client resource,
thus they keeps no handle value.
This commit adds the series of functions to separate these operations,
according to divide-and-conquer methodology.
Link: https://lore.kernel.org/r/20260429093449.160545-5-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
parameters
This change is a preparation for future changes. The added helper function
will be reused in the changes to fill iso_resource parameters according to
the users' request.
Link: https://lore.kernel.org/r/20260429093449.160545-4-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The add_client_resource() function checks the type of client resource
every time to be called. If the type is for iso_resource, it schedules
work item.
However, the iso_resource client resource is only added by the call of
init_iso_resource(). There is no need to check the type every time adding
any client resource.
Link: https://lore.kernel.org/r/20260429093449.160545-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The add_client_resource() function returns zero at success or negative
value at error. The critical section is already protected by
scoped_guard() macro. In this case, the programming pattern of early
return improves code readability.
Link: https://lore.kernel.org/r/20260429093449.160545-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
This is the exact same thing as the 'alloc_obj()' version, only much
smaller because there are a lot fewer users of the *alloc_flex()
interface.
As with alloc_obj() version, this was done entirely with mindless brute
force, using the same script, except using 'flex' in the pattern rather
than 'objs*'.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
|
|
This was done entirely with mindless brute force, using
git grep -l '\<k[vmz]*alloc_objs*(.*, GFP_KERNEL)' |
xargs sed -i 's/\(alloc_objs*(.*\), GFP_KERNEL)/\1)/'
to convert the new alloc_obj() users that had a simple GFP_KERNEL
argument to just drop that argument.
Note that due to the extreme simplicity of the scripting, any slightly
more complex cases spread over multiple lines would not be triggered:
they definitely exist, but this covers the vast bulk of the cases, and
the resulting diff is also then easier to check automatically.
For the same reason the 'flex' versions will be done as a separate
conversion.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
|
|
This is the result of running the Coccinelle script from
scripts/coccinelle/api/kmalloc_objs.cocci. The script is designed to
avoid scalar types (which need careful case-by-case checking), and
instead replace kmalloc-family calls that allocate struct or union
object instances:
Single allocations: kmalloc(sizeof(TYPE), ...)
are replaced with: kmalloc_obj(TYPE, ...)
Array allocations: kmalloc_array(COUNT, sizeof(TYPE), ...)
are replaced with: kmalloc_objs(TYPE, COUNT, ...)
Flex array allocations: kmalloc(struct_size(PTR, FAM, COUNT), ...)
are replaced with: kmalloc_flex(*PTR, FAM, COUNT, ...)
(where TYPE may also be *VAR)
The resulting allocations no longer return "void *", instead returning
"TYPE *".
Signed-off-by: Kees Cook <kees@kernel.org>
|
|
This is minor code refactoring to add a flag member to the isochronous
context structure. At present, it is used only for the option to drop
packets when the context header overflows.
Link: https://lore.kernel.org/r/20260117142823.440811-6-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The fw_iso_callback union was added by a commit ebe4560ed5c ("firewire:
Remove function callback casts") to remove function pointer cast.
That change affected the cdev layer of the core code, but it is more
convenient for fw_iso_context_create() to accept the union directly.
This commit renames and changes the existing function to take the union
argument, and add static inline wrapper functions as variants.
Link: https://lore.kernel.org/r/20260117142823.440811-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
There is a long discussion about the use of private field in page
structure between Linux kernel developers.
This commit stop using page private to store DMA mapping address for
isochronous context, to prepare for mm future change.
Link: https://lore.kernel.org/r/20260110013911.19160-6-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
There is a restriction that the userspace client associated to a file
descriptor can hold one isochronous context. The client-level spinning
lock is used to guarantee it, however the lock is also used for
multi-purposes. Additionally, there is no need to use this type of lock,
and the mutex is available, instead.
Link: https://lore.kernel.org/r/20260110013911.19160-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394
Pull firewire updates from Takashi Sakamoto:
"This update includes the following changes:
- Removal of the deprecated debug parameter from firewire-ohci module
- Replacement of the module-local workqueue in 1394 OHCI PCI driver
with a companion IRQ thread
- Refactoring of bus management code
- Additional minor code cleanup
The existing tracepoints serve as an alternative to the removed debug
parameter. The use of IRQ thread is experimental, as it handles 1394
OHCI SelfIDComplete event only. It may be replaced in the future
releases with another approach; e.g. by providing workqueue from core
functionality"
* tag 'firewire-updates-6.18' of git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394: (43 commits)
firewire: core: fix undefined reference error in ARM EABI
Revert "firewire: core: disable bus management work temporarily during updating topology"
Revert "firewire: core: shrink critical section of fw_card spinlock in bm_work"
firewire: core: suppress overflow warning when computing jiffies from isochronous cycle
firewire: core: minor code refactoring to delete useless local variable
firewire: core; eliminate pick_me goto label
firewire: core: code refactoring to split contention procedure for bus manager
firewire: core: code refactoring for the case of generation mismatch
firewire: core: use switch statement to evaluate transaction result to CSR_BUS_MANAGER_ID
firewire: core: remove useless generation check
firewire: core: use struct_size and flex_array_size in ioctl_add_descriptor
firewire: core: shrink critical section of fw_card spinlock in bm_work
firewire: core: disable bus management work temporarily during updating topology
firewire: core: schedule bm_work item outside of spin lock
firewire: core: annotate fw_destroy_nodes with must-hold-lock
firewire: core: use spin lock specific to timer for split transaction
firewire: core: use spin lock specific to transaction
firewire: core: use spin lock specific to topology map
firewire: core: maintain phy packet receivers locally in cdev layer
firewire: core: use scoped_guard() to manage critical section to update topology
...
|
|
In kernel v6.5, several functions were added to the cdev layer. This
required updating the default version of subsystem ABI up to 6, but
this requirement was overlooked.
This commit updates the version accordingly.
Fixes: 6add87e9764d ("firewire: cdev: add new version of ABI to notify time stamp at request/response subaction of transaction#")
Link: https://lore.kernel.org/r/20250920025148.163402-1-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
Use struct_size() to determine the memory needed for a new 'struct
descriptor_resource' and flex_array_size() to calculate the number of
bytes to copy from userspace. This removes the hardcoded size (4 bytes)
for the 'u32 data[]' entries.
No functional changes intended.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Link: https://lore.kernel.org/r/20250916122143.2459993-3-thorsten.blum@linux.dev
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The list of receivers for phy packet is used only by cdev layer, while it
is maintained as a member of fw_card structure.
This commit maintains the list locally in cdev layer.
Link: https://lore.kernel.org/r/20250915234747.915922-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
The pattern of calling either time_before64() or time_after64() with
get_jiffies_64() can be replaced with the corresponding helper macros.
Link: https://lore.kernel.org/r/20250915024232.851955-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
There are some macros available to convert usecs, msecs, and secs into
jiffies count.
Link: https://lore.kernel.org/r/20250915024232.851955-2-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
|
A commit 60b2ebf48526 ("workqueue: Introduce from_work() helper for cleaner
callback declarations") introduces a new macro to retrieve a poiner for the
parent structure of the work item. It is convenient to reduce input text.
This commit uses the macro in core functionalities.
Link: https://lore.kernel.org/r/20250608233808.202355-3-o-takashi@sakamocchi.jp
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
|
