Re: [PATCH v3 1/6] rfcomm: Take proper tty_struct references

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/26/2013 08:50 AM, Gianluca Anzolin wrote:
On Fri, Jul 26, 2013 at 08:07:30AM -0400, Peter Hurley wrote:
On 07/25/2013 02:32 PM, Gianluca Anzolin wrote:

  	rfcomm_dlc_lock(dlc);
  	tty->driver_data = dev;
-	dev->port.tty = tty;
+	tty_port_tty_set(&dev->port, tty);

Although strictly speaking, this is correct, I would drop this change
because its functionality is replaced in 4/6 with the call to tty_port_open().
If you want, you could note in the commit message that the
raw assignments in rfcomm_tty_open/close are addressed in commit
'rfcomm: Implement .activate, .shutdown and .carrier_raised methods'.

Ok I will do that.


[ BTW, you remove this line in 3/6 but it's needed until 4/6]

Oh, I overlooked that "detail" when I changed the order of the patches...

Same thing just happened to me with a series of mine.


  	rfcomm_dlc_unlock(dlc);
  	set_bit(RFCOMM_TTY_ATTACHED, &dev->flags);

@@ -742,7 +747,7 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)

  		rfcomm_dlc_lock(dev->dlc);
  		tty->driver_data = NULL;
-		dev->port.tty = NULL;
+		tty_port_tty_set(&dev->port, NULL);

Similarly, the call to tty_port_close() in 4/6 replaces this functionality.

Regards,
Peter Hurley


Ok there is a need for a v4 after all.

Btw did you look at the RFC patch about the dev_add nested locks?
Do you think it's acceptable? I tried to add the device to the list at the end
of the function but the fact is that the dlc callbacks need dev->id so I had to
allocate it or set it to something not valid, like -1.
>
> I didn't also get any reply about the skb_queue_purge patch, I hope it's ok

I'm testing the whole patch series now. Assuming nothing breaks and v4 changes
are minimal, I'll give the whole series Reviewed-by: and Tested-by: tags.

When I'm done with that, I'll review the RFC patch.

For v4, you should direct To: Gustavo Padovan, cc: me instead, and add
cc's for Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> and Jiri Slaby
<jslaby@xxxxxxx>

Regards,
Peter Hurley


For future reference, for a series like this you might want to consider
adding a cover letter 0/6 (with git format-patch --cover-letter) with an
overview of the whole series.

Cover letters give you an opportunity to explain the motivation
(rfcomm crashes on device disconnect because port reference counting
was broken), explain your solution (implement rfcomm as a proper tty_port),
and allows reviewers to comment on the series as whole (Good work :).
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux