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: fcabbf40fa ("firewire: core: move allocation/reallocation paths into specific branch after isoc resource management in cdev")
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
The fw_iso_callback union was added by a commit ebe4560ed5 ("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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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: 6add87e976 ("firewire: cdev: add new version of ABI to notify time stamp at request/response subaction of transaction#")
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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 <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
A commit 60b2ebf485 ("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/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
Use the `DEFINE_RAW_FLEX()` helper for an on-stack definition of
a flexible structure where the size of the flexible-array member
is known at compile-time, and refactor the rest of the code,
accordingly.
So, with these changes, fix the following warning:
drivers/firewire/core-cdev.c:1141:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
Signed-off-by: Gustavo A. R. Silva <[email protected]>
Link: https://lore.kernel.org/r/Z9NcB81yfPo-8o0h@kspp
Signed-off-by: Takashi Sakamoto <[email protected]>
no_llseek had been defined to NULL two years ago, in commit 868941b144
("fs: remove no_llseek")
To quote that commit,
At -rc1 we'll need do a mechanical removal of no_llseek -
git grep -l -w no_llseek | grep -v porting.rst | while read i; do
sed -i '/\<no_llseek\>/d' $i
done
would do it.
Unfortunately, that hadn't been done. Linus, could you do that now, so
that we could finally put that thing to rest? All instances are of the
form
.llseek = no_llseek,
so it's obviously safe.
Signed-off-by: Al Viro <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
In the former commits, the callback of isochronous context runs on work
process, thus no need to use atomic memory allocation.
This commit replaces GFP_ATOMIC with GCP_KERNEL in the callback for user
client.
Tested-by: Edmund Raile <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
In core function, the instances of some client resource structures are
maintained by IDR. As of kernel v6.0, IDR has been superseded by XArray
and deprecated.
This commit replaces the usage of IDR with XArray to maintain the
resource instances. The instance of XArray is allocated per client with
XA_FLAGS_ALLOC1 so that the index of allocated entry is greater than zero
and returns to user space client as handle of the resource.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
This commit is a preparation to use xa_for_each() macro. Current
implementation uses idr_for_each() function and has a disadvantage to
replace with the macro. The IDR framework has idr_for_each_entry() macro
for the similar purpose. This commit replace the function with the
macro with minor code refactoring.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
It depends on the function assigned to release member to identify
resource structure.
This commit adds a helper function to identify iso_resource structure.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
All of local resource structure commonly have data of client_resource type
in its first member. This design sometimes requires usage of
container_of to retrieve parent structure by the first member.
This commit adds some helper functions for this purpose.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
Current implementation checks and validates the result to find resource
entry two times. It is redundant.
This commit refactors the redundancy.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
A commit d8527cab6c ("firewire: cdev: implement new event to notify
response subaction with time stamp") adds an additional case,
FW_CDEV_EVENT_RESPONSE2, into switch statement in complete_transaction().
However, the range of block is beyond to the case label and reaches
neibour default label.
This commit corrects the range of block. Fortunately, it has few impacts
in practice since the local variable in the scope under the label is not
used in codes under default label.
Fixes: d8527cab6c ("firewire: cdev: implement new event to notify response subaction with time stamp")
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
The core functions uses spinlock in instance of fw_card structure to
protect concurrent access to properties in the instance.
This commit uses guard macro to maintain the spinlock.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
The core function maintains clients to receive phy configuration packets
by list in the instance of fw_card. The concurrent access to the list is
protected by spinlock in the instance.
This commit uses guard macro to maintain the list.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
The core function allows one isochronous contexts per userspace client.
The concurrent access to the context is protected by spinlock in the
instance of client.
This commit uses guard macro to maintain the spinlock.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
The core function provides UAPI to maintain isochronous resources allocated
by userspace clients across bus resets automatically. The resources are
maintained by IDR and the concurrent access to it is protected by spinlock
in the instance of client.
This commit uses guard macro to maintain the spinlock.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
The core function maintains events to userspace by list in the instance of
client. The concurrent access to the list is protected by spinlock in
the instance.
This commit uses guard macro to maintain the spinlock.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
The core function provides an operation for userspace application to
retrieve current value of CYCLE_TIMER register with several types of
system time. In the operation, local interrupt is disables so that the
access of the register and ktime are done atomically.
This commit uses guard macro to disable/enable local interrupts.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
The core function maintains the instance of fw_device structure by IDR.
The concurrent access to IDR is protected by static read/write semaphore.
The semaphore is also utilized to protect concurrent access to the
content of configuration ROM cached to the instance so that the cache is
swapped to the latest one.
This commit uses guard macro to maintain the mutex.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
The core function maintains userspace clients by the list in fw_device
object associated to the operated character device. The concurrent
access to the list is protected by mutex in the object.
This commit uses guard macro to maintain the mutex.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
The asynchronous transmission of phy packet is initiated on one of 1394
OHCI controller, however the existing tracepoints events has the lack of
data about it.
This commit adds card_index member into event structure to store the index
of host controller in use, and prints it.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
The asynchronous transaction is initiated on one of 1394 OHCI
controller, however the existing tracepoints events has the lack of data
about it.
This commit adds card_index member into event structure to store the index
of host controller in use, and prints it.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
In IEEE 1394 bus, the type of asynchronous packet without any offset to
node address space is called as phy packet. The destination of packet is
IEEE 1394 phy itself. This type of packet is used for several purposes,
mainly for selfID at the state of bus reset, to force selection of root
node, and to adjust gap count.
This commit adds tracepoints events for the type of asynchronous outbound
packet. Like asynchronous outbound transaction packets, a pair of events
are added to trace initiation and completion of transmission.
In the case that the phy packet is sent by kernel API, the match between
the initiation and completion is not so easy, since the data of
'struct fw_packet' is allocated statically. In the case that it is sent by
userspace applications via cdev, the match is easy, since the data is
allocated per each.
This example is for Remote Access Packet by lsfirewirephy command in
linux-firewire-utils:
async_phy_outbound_initiate: \
packet=0xffff89fb34e42e78 generation=1 first_quadlet=0x00148200 \
second_quadlet=0xffeb7dff
async_phy_outbound_complete: \
packet=0xffff89fb34e42e78 generation=1 status=1 timestamp=0x0619
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Sakamoto <[email protected]>
Takashi Sakamoto