Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

misc/sgi-xp: Replace in_interrupt() usage

The usage of in_interrupt() in xpc_partition_disengaged() is clearly
intended to avoid canceling the timeout timer when the function is invoked
from the timer callback.

While in_interrupt() is deprecated and ill defined as it does not provide
what the name suggests it catches the intended case.

Add an argument to xpc_partition_disengaged() which is true if called
from timer and otherwise false.
Use del_timer_sync() instead of del_singleshot_timer_sync() which is the
same thing.

Note: This does not prevent reentrancy into the function as the function
has no concurrency control and timer callback and regular task context
callers can happen concurrently on different CPUs or the timer can
interrupt the task context before it is able to cancel it.

While the only driver which is providing the arch_xpc_ops callbacks
(xpc_uv) seems not to have a reentrancy problem and the only negative
effect would be a double dev_info() entry in dmesg, the whole mechanism is
conceptually broken.

But that's not subject of this cleanup endeavour and left as an exercise to
the folks who might have interest to make that code fully correct.

[bigeasy: Add the argument, use del_timer_sync().]

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Cliff Whickman <cpw@sgi.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Robin Holt <robinmholt@gmail.com>
Cc: Steve Wahl <steve.wahl@hpe.com>
Cc: Dimitri Sivanich <dimitri.sivanich@hpe.com>
Cc: Russ Anderson <russ.anderson@hpe.com>
Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Link: https://lore.kernel.org/r/20201119103151.ppo45mj53ulbxjx4@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Thomas Gleixner and committed by
Greg Kroah-Hartman
997754f1 a73a0712

+17 -6
+1
drivers/misc/sgi-xp/xpc.h
··· 634 634 extern void xpc_teardown_rsvd_page(void); 635 635 extern int xpc_identify_activate_IRQ_sender(void); 636 636 extern int xpc_partition_disengaged(struct xpc_partition *); 637 + extern int xpc_partition_disengaged_from_timer(struct xpc_partition *part); 637 638 extern enum xp_retval xpc_mark_partition_active(struct xpc_partition *); 638 639 extern void xpc_mark_partition_inactive(struct xpc_partition *); 639 640 extern void xpc_discovery(void);
+1 -1
drivers/misc/sgi-xp/xpc_main.c
··· 179 179 180 180 DBUG_ON(time_is_after_jiffies(part->disengage_timeout)); 181 181 182 - (void)xpc_partition_disengaged(part); 182 + xpc_partition_disengaged_from_timer(part); 183 183 184 184 DBUG_ON(part->disengage_timeout != 0); 185 185 DBUG_ON(xpc_arch_ops.partition_engaged(XPC_PARTID(part)));
+15 -5
drivers/misc/sgi-xp/xpc_partition.c
··· 262 262 * from us. Though we requested the remote partition to deactivate with regard 263 263 * to us, we really only need to wait for the other side to disengage from us. 264 264 */ 265 - int 266 - xpc_partition_disengaged(struct xpc_partition *part) 265 + static int __xpc_partition_disengaged(struct xpc_partition *part, 266 + bool from_timer) 267 267 { 268 268 short partid = XPC_PARTID(part); 269 269 int disengaged; ··· 289 289 } 290 290 part->disengage_timeout = 0; 291 291 292 - /* cancel the timer function, provided it's not us */ 293 - if (!in_interrupt()) 294 - del_singleshot_timer_sync(&part->disengage_timer); 292 + /* Cancel the timer function if not called from it */ 293 + if (!from_timer) 294 + del_timer_sync(&part->disengage_timer); 295 295 296 296 DBUG_ON(part->act_state != XPC_P_AS_DEACTIVATING && 297 297 part->act_state != XPC_P_AS_INACTIVE); ··· 301 301 xpc_arch_ops.cancel_partition_deactivation_request(part); 302 302 } 303 303 return disengaged; 304 + } 305 + 306 + int xpc_partition_disengaged(struct xpc_partition *part) 307 + { 308 + return __xpc_partition_disengaged(part, false); 309 + } 310 + 311 + int xpc_partition_disengaged_from_timer(struct xpc_partition *part) 312 + { 313 + return __xpc_partition_disengaged(part, true); 304 314 } 305 315 306 316 /*