On 09-02-23 09:33 pm, Alan Stern wrote:
On Thu, Feb 09, 2023 at 09:13:37PM +0530, Prashanth K wrote:
On 09-02-23 08:39 pm, Alan Stern wrote:
You should consider having _two_ spinlocks: One in the gs_port structure
(the way it is now) and a separate global lock. The first would be used
in situations where you know you have a valid pointer. The second would
be used in situations where you don't know if the pointer is non-NULL
or where you are changing the pointer's value.
Lets say we replaced the existing spinlock in gserial_resume and
gserial_disconnect with a new static spinlock, and kept the spinlocks in
other functions unchanged. In that case, wouldn't it cause additional race
conditions as we are using 2 different locks.
Not race conditions, but possibilities for deadlock.
Indeed, you would have to be very careful about avoiding deadlock
scenarios. In particular, you would have to ensure that the code never
tries to acquire the global spinlock while already holding one of the
per-port spinlocks.
Alan Stern
Hi Alan, instead of doing these and causing potential regressions, can
we just have the null pointer check which i suggested in the beginning?
The major concern was that port might become null after the null pointer
check. We mark gser->ioport as null pointer in gserial_disconnect, and
in gserial_resume we copy the gser->ioport to *port in the beginning.
struct gs_port *port = gser->ioport;
And hence it wont cause null pointer deref after the check as we don't
de-reference anything from gser->ioport afterwards. We only use the
local pointer *port afterwards.
Thanks,
Prashanth K
