linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* linux-next: manual merge of the tty tree with the tty.current tree
@ 2016-02-08  2:16 Stephen Rothwell
  2016-02-08  2:21 ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Rothwell @ 2016-02-08  2:16 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-next, linux-kernel, Peter Hurley

Hi Greg,

Today's linux-next merge of the tty tree got a conflict in:

  drivers/tty/tty_io.c

between commit:

  e9036d066236 ("tty: Drop krefs for interrupted tty lock")

from the tty.current tree and commit:

  d6203d0c7b73 ("tty: Refactor tty_open()")

from the tty tree.

I fixed it up (I think - see below) and can carry the fix as necessary
(no action is required).

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/tty/tty_io.c
index a7eacef1bd22,8d26ed79bb4c..000000000000
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@@ -2004,6 -2009,69 +2009,68 @@@ static struct tty_driver *tty_lookup_dr
  }
  
  /**
+  *	tty_open_by_driver	-	open a tty device
+  *	@device: dev_t of device to open
+  *	@inode: inode of device file
+  *	@filp: file pointer to tty
+  *
+  *	Performs the driver lookup, checks for a reopen, or otherwise
+  *	performs the first-time tty initialization.
+  *
+  *	Returns the locked initialized or re-opened &tty_struct
+  *
+  *	Claims the global tty_mutex to serialize:
+  *	  - concurrent first-time tty initialization
+  *	  - concurrent tty driver removal w/ lookup
+  *	  - concurrent tty removal from driver table
+  */
+ static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
+ 					     struct file *filp)
+ {
+ 	struct tty_struct *tty;
+ 	struct tty_driver *driver = NULL;
+ 	int index = -1;
+ 	int retval;
+ 
+ 	mutex_lock(&tty_mutex);
+ 	driver = tty_lookup_driver(device, filp, &index);
+ 	if (IS_ERR(driver)) {
+ 		mutex_unlock(&tty_mutex);
+ 		return ERR_CAST(driver);
+ 	}
+ 
+ 	/* check whether we're reopening an existing tty */
+ 	tty = tty_driver_lookup_tty(driver, inode, index);
+ 	if (IS_ERR(tty)) {
+ 		mutex_unlock(&tty_mutex);
+ 		goto out;
+ 	}
+ 
+ 	if (tty) {
+ 		mutex_unlock(&tty_mutex);
+ 		retval = tty_lock_interruptible(tty);
++		tty_kref_put(tty);  /* drop kref from tty_driver_lookup_tty() */
+ 		if (retval) {
+ 			if (retval == -EINTR)
+ 				retval = -ERESTARTSYS;
+ 			tty = ERR_PTR(retval);
+ 			goto out;
+ 		}
 -		/* safe to drop the kref from tty_driver_lookup_tty() */
 -		tty_kref_put(tty);
+ 		retval = tty_reopen(tty);
+ 		if (retval < 0) {
+ 			tty_unlock(tty);
+ 			tty = ERR_PTR(retval);
+ 		}
+ 	} else { /* Returns with the tty_lock held for now */
+ 		tty = tty_init_dev(driver, index);
+ 		mutex_unlock(&tty_mutex);
+ 	}
+ out:
+ 	tty_driver_kref_put(driver);
+ 	return tty;
+ }
+ 
+ /**
   *	tty_open		-	open a tty device
   *	@inode: inode of device file
   *	@filp: file pointer to tty

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: linux-next: manual merge of the tty tree with the tty.current tree
  2016-02-08  2:16 linux-next: manual merge of the tty tree with the tty.current tree Stephen Rothwell
@ 2016-02-08  2:21 ` Greg KH
  2016-02-08  2:53   ` Peter Hurley
  2016-04-01  0:23   ` Peter Hurley
  0 siblings, 2 replies; 6+ messages in thread
From: Greg KH @ 2016-02-08  2:21 UTC (permalink / raw)
  To: Stephen Rothwell, Peter Hurley; +Cc: linux-next, linux-kernel

On Mon, Feb 08, 2016 at 01:16:29PM +1100, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in:
> 
>   drivers/tty/tty_io.c
> 
> between commit:
> 
>   e9036d066236 ("tty: Drop krefs for interrupted tty lock")
> 
> from the tty.current tree and commit:
> 
>   d6203d0c7b73 ("tty: Refactor tty_open()")
> 
> from the tty tree.
> 
> I fixed it up (I think - see below) and can carry the fix as necessary
> (no action is required).
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc drivers/tty/tty_io.c
> index a7eacef1bd22,8d26ed79bb4c..000000000000
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@@ -2004,6 -2009,69 +2009,68 @@@ static struct tty_driver *tty_lookup_dr
>   }
>   
>   /**
> +  *	tty_open_by_driver	-	open a tty device
> +  *	@device: dev_t of device to open
> +  *	@inode: inode of device file
> +  *	@filp: file pointer to tty
> +  *
> +  *	Performs the driver lookup, checks for a reopen, or otherwise
> +  *	performs the first-time tty initialization.
> +  *
> +  *	Returns the locked initialized or re-opened &tty_struct
> +  *
> +  *	Claims the global tty_mutex to serialize:
> +  *	  - concurrent first-time tty initialization
> +  *	  - concurrent tty driver removal w/ lookup
> +  *	  - concurrent tty removal from driver table
> +  */
> + static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
> + 					     struct file *filp)
> + {
> + 	struct tty_struct *tty;
> + 	struct tty_driver *driver = NULL;
> + 	int index = -1;
> + 	int retval;
> + 
> + 	mutex_lock(&tty_mutex);
> + 	driver = tty_lookup_driver(device, filp, &index);
> + 	if (IS_ERR(driver)) {
> + 		mutex_unlock(&tty_mutex);
> + 		return ERR_CAST(driver);
> + 	}
> + 
> + 	/* check whether we're reopening an existing tty */
> + 	tty = tty_driver_lookup_tty(driver, inode, index);
> + 	if (IS_ERR(tty)) {
> + 		mutex_unlock(&tty_mutex);
> + 		goto out;
> + 	}
> + 
> + 	if (tty) {
> + 		mutex_unlock(&tty_mutex);
> + 		retval = tty_lock_interruptible(tty);
> ++		tty_kref_put(tty);  /* drop kref from tty_driver_lookup_tty() */
> + 		if (retval) {
> + 			if (retval == -EINTR)
> + 				retval = -ERESTARTSYS;
> + 			tty = ERR_PTR(retval);
> + 			goto out;
> + 		}
>  -		/* safe to drop the kref from tty_driver_lookup_tty() */
>  -		tty_kref_put(tty);
> + 		retval = tty_reopen(tty);
> + 		if (retval < 0) {
> + 			tty_unlock(tty);
> + 			tty = ERR_PTR(retval);
> + 		}
> + 	} else { /* Returns with the tty_lock held for now */
> + 		tty = tty_init_dev(driver, index);
> + 		mutex_unlock(&tty_mutex);
> + 	}
> + out:
> + 	tty_driver_kref_put(driver);
> + 	return tty;
> + }
> + 
> + /**
>    *	tty_open		-	open a tty device
>    *	@inode: inode of device file
>    *	@filp: file pointer to tty

Peter warned me this was going to happen...

Peter, is the merge above correct?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: linux-next: manual merge of the tty tree with the tty.current tree
  2016-02-08  2:21 ` Greg KH
@ 2016-02-08  2:53   ` Peter Hurley
  2016-04-01  0:23   ` Peter Hurley
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Hurley @ 2016-02-08  2:53 UTC (permalink / raw)
  To: Greg KH, Stephen Rothwell; +Cc: linux-next, linux-kernel

On 02/07/2016 06:21 PM, Greg KH wrote:
> Peter, is the merge above correct?

Yes, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: linux-next: manual merge of the tty tree with the tty.current tree
  2016-02-08  2:21 ` Greg KH
  2016-02-08  2:53   ` Peter Hurley
@ 2016-04-01  0:23   ` Peter Hurley
  2016-04-01  0:47     ` [PATCH] tty: Fix merge of "tty: Refactor tty_open()" Peter Hurley
  2016-04-01  3:49     ` linux-next: manual merge of the tty tree with the tty.current tree Greg KH
  1 sibling, 2 replies; 6+ messages in thread
From: Peter Hurley @ 2016-04-01  0:23 UTC (permalink / raw)
  To: Greg KH; +Cc: Stephen Rothwell, linux-next, Linux kernel mailing list

On Sun, Feb 7, 2016 at 6:21 PM, Greg KH <greg@kroah.com> wrote:
> On Mon, Feb 08, 2016 at 01:16:29PM +1100, Stephen Rothwell wrote:
>> Hi Greg,
>>
>> Today's linux-next merge of the tty tree got a conflict in:
>>
>>   drivers/tty/tty_io.c
>>
>> between commit:
>>
>>   e9036d066236 ("tty: Drop krefs for interrupted tty lock")
>>
>> from the tty.current tree and commit:
>>
>>   d6203d0c7b73 ("tty: Refactor tty_open()")
>>
>> from the tty tree.
>>
>> I fixed it up (I think - see below) and can carry the fix as necessary
>> (no action is required).
>>
>> --
>> Cheers,
>> Stephen Rothwell
>>
>> diff --cc drivers/tty/tty_io.c
>> index a7eacef1bd22,8d26ed79bb4c..000000000000
>> --- a/drivers/tty/tty_io.c
>> +++ b/drivers/tty/tty_io.c
>> @@@ -2004,6 -2009,69 +2009,68 @@@ static struct tty_driver *tty_lookup_dr
>>   }
>>
>>   /**
>> +  *  tty_open_by_driver      -       open a tty device
>> +  *  @device: dev_t of device to open
>> +  *  @inode: inode of device file
>> +  *  @filp: file pointer to tty
>> +  *
>> +  *  Performs the driver lookup, checks for a reopen, or otherwise
>> +  *  performs the first-time tty initialization.
>> +  *
>> +  *  Returns the locked initialized or re-opened &tty_struct
>> +  *
>> +  *  Claims the global tty_mutex to serialize:
>> +  *    - concurrent first-time tty initialization
>> +  *    - concurrent tty driver removal w/ lookup
>> +  *    - concurrent tty removal from driver table
>> +  */
>> + static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
>> +                                          struct file *filp)
>> + {
>> +     struct tty_struct *tty;
>> +     struct tty_driver *driver = NULL;
>> +     int index = -1;
>> +     int retval;
>> +
>> +     mutex_lock(&tty_mutex);
>> +     driver = tty_lookup_driver(device, filp, &index);
>> +     if (IS_ERR(driver)) {
>> +             mutex_unlock(&tty_mutex);
>> +             return ERR_CAST(driver);
>> +     }
>> +
>> +     /* check whether we're reopening an existing tty */
>> +     tty = tty_driver_lookup_tty(driver, inode, index);
>> +     if (IS_ERR(tty)) {
>> +             mutex_unlock(&tty_mutex);
>> +             goto out;
>> +     }
>> +
>> +     if (tty) {
>> +             mutex_unlock(&tty_mutex);
>> +             retval = tty_lock_interruptible(tty);
>> ++            tty_kref_put(tty);  /* drop kref from tty_driver_lookup_tty() */
>> +             if (retval) {
>> +                     if (retval == -EINTR)
>> +                             retval = -ERESTARTSYS;
>> +                     tty = ERR_PTR(retval);
>> +                     goto out;
>> +             }
>>  -            /* safe to drop the kref from tty_driver_lookup_tty() */
>>  -            tty_kref_put(tty);
>> +             retval = tty_reopen(tty);
>> +             if (retval < 0) {
>> +                     tty_unlock(tty);
>> +                     tty = ERR_PTR(retval);
>> +             }
>> +     } else { /* Returns with the tty_lock held for now */
>> +             tty = tty_init_dev(driver, index);
>> +             mutex_unlock(&tty_mutex);
>> +     }
>> + out:
>> +     tty_driver_kref_put(driver);
>> +     return tty;
>> + }
>> +
>> + /**
>>    *  tty_open                -       open a tty device
>>    *  @inode: inode of device file
>>    *  @filp: file pointer to tty
>
> Peter warned me this was going to happen...
>
> Peter, is the merge above correct?

Greg, this merge correction did not make it into 4.6-rc1.

Was I supposed to send a separate patch for this merge change?
Should I now?

Regards,
Peter Hurley

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] tty: Fix merge of "tty: Refactor tty_open()"
  2016-04-01  0:23   ` Peter Hurley
@ 2016-04-01  0:47     ` Peter Hurley
  2016-04-01  3:49     ` linux-next: manual merge of the tty tree with the tty.current tree Greg KH
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Hurley @ 2016-04-01  0:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, linux-next, Stephen Rothwell, Peter Hurley

Commit e9036d066236 ("tty: Drop krefs for interrupted tty lock")
fixed a tty reference counting problem introduced in
commit 0bfd464d3fdd ("tty: Wait interruptibly for tty lock on reopen"),
so v4.5.0 is correct.

However, commit d6203d0c7b73 ("tty: Refactor tty_open()") moved the
relevant code for 4.6-rc1; correct the merge.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 8d26ed7..c14c45f 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2049,14 +2049,13 @@ static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
 	if (tty) {
 		mutex_unlock(&tty_mutex);
 		retval = tty_lock_interruptible(tty);
+		tty_kref_put(tty);  /* drop kref from tty_driver_lookup_tty() */
 		if (retval) {
 			if (retval == -EINTR)
 				retval = -ERESTARTSYS;
 			tty = ERR_PTR(retval);
 			goto out;
 		}
-		/* safe to drop the kref from tty_driver_lookup_tty() */
-		tty_kref_put(tty);
 		retval = tty_reopen(tty);
 		if (retval < 0) {
 			tty_unlock(tty);
-- 
2.8.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: linux-next: manual merge of the tty tree with the tty.current tree
  2016-04-01  0:23   ` Peter Hurley
  2016-04-01  0:47     ` [PATCH] tty: Fix merge of "tty: Refactor tty_open()" Peter Hurley
@ 2016-04-01  3:49     ` Greg KH
  1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2016-04-01  3:49 UTC (permalink / raw)
  To: Peter Hurley; +Cc: Stephen Rothwell, linux-next, Linux kernel mailing list

On Thu, Mar 31, 2016 at 05:23:27PM -0700, Peter Hurley wrote:
> On Sun, Feb 7, 2016 at 6:21 PM, Greg KH <greg@kroah.com> wrote:
> > On Mon, Feb 08, 2016 at 01:16:29PM +1100, Stephen Rothwell wrote:
> >> Hi Greg,
> >>
> >> Today's linux-next merge of the tty tree got a conflict in:
> >>
> >>   drivers/tty/tty_io.c
> >>
> >> between commit:
> >>
> >>   e9036d066236 ("tty: Drop krefs for interrupted tty lock")
> >>
> >> from the tty.current tree and commit:
> >>
> >>   d6203d0c7b73 ("tty: Refactor tty_open()")
> >>
> >> from the tty tree.
> >>
> >> I fixed it up (I think - see below) and can carry the fix as necessary
> >> (no action is required).
> >>
> >> --
> >> Cheers,
> >> Stephen Rothwell
> >>
> >> diff --cc drivers/tty/tty_io.c
> >> index a7eacef1bd22,8d26ed79bb4c..000000000000
> >> --- a/drivers/tty/tty_io.c
> >> +++ b/drivers/tty/tty_io.c
> >> @@@ -2004,6 -2009,69 +2009,68 @@@ static struct tty_driver *tty_lookup_dr
> >>   }
> >>
> >>   /**
> >> +  *  tty_open_by_driver      -       open a tty device
> >> +  *  @device: dev_t of device to open
> >> +  *  @inode: inode of device file
> >> +  *  @filp: file pointer to tty
> >> +  *
> >> +  *  Performs the driver lookup, checks for a reopen, or otherwise
> >> +  *  performs the first-time tty initialization.
> >> +  *
> >> +  *  Returns the locked initialized or re-opened &tty_struct
> >> +  *
> >> +  *  Claims the global tty_mutex to serialize:
> >> +  *    - concurrent first-time tty initialization
> >> +  *    - concurrent tty driver removal w/ lookup
> >> +  *    - concurrent tty removal from driver table
> >> +  */
> >> + static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
> >> +                                          struct file *filp)
> >> + {
> >> +     struct tty_struct *tty;
> >> +     struct tty_driver *driver = NULL;
> >> +     int index = -1;
> >> +     int retval;
> >> +
> >> +     mutex_lock(&tty_mutex);
> >> +     driver = tty_lookup_driver(device, filp, &index);
> >> +     if (IS_ERR(driver)) {
> >> +             mutex_unlock(&tty_mutex);
> >> +             return ERR_CAST(driver);
> >> +     }
> >> +
> >> +     /* check whether we're reopening an existing tty */
> >> +     tty = tty_driver_lookup_tty(driver, inode, index);
> >> +     if (IS_ERR(tty)) {
> >> +             mutex_unlock(&tty_mutex);
> >> +             goto out;
> >> +     }
> >> +
> >> +     if (tty) {
> >> +             mutex_unlock(&tty_mutex);
> >> +             retval = tty_lock_interruptible(tty);
> >> ++            tty_kref_put(tty);  /* drop kref from tty_driver_lookup_tty() */
> >> +             if (retval) {
> >> +                     if (retval == -EINTR)
> >> +                             retval = -ERESTARTSYS;
> >> +                     tty = ERR_PTR(retval);
> >> +                     goto out;
> >> +             }
> >>  -            /* safe to drop the kref from tty_driver_lookup_tty() */
> >>  -            tty_kref_put(tty);
> >> +             retval = tty_reopen(tty);
> >> +             if (retval < 0) {
> >> +                     tty_unlock(tty);
> >> +                     tty = ERR_PTR(retval);
> >> +             }
> >> +     } else { /* Returns with the tty_lock held for now */
> >> +             tty = tty_init_dev(driver, index);
> >> +             mutex_unlock(&tty_mutex);
> >> +     }
> >> + out:
> >> +     tty_driver_kref_put(driver);
> >> +     return tty;
> >> + }
> >> +
> >> + /**
> >>    *  tty_open                -       open a tty device
> >>    *  @inode: inode of device file
> >>    *  @filp: file pointer to tty
> >
> > Peter warned me this was going to happen...
> >
> > Peter, is the merge above correct?
> 
> Greg, this merge correction did not make it into 4.6-rc1.
> 
> Was I supposed to send a separate patch for this merge change?
> Should I now?

The patch you sent should be fine, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-04-01  3:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-08  2:16 linux-next: manual merge of the tty tree with the tty.current tree Stephen Rothwell
2016-02-08  2:21 ` Greg KH
2016-02-08  2:53   ` Peter Hurley
2016-04-01  0:23   ` Peter Hurley
2016-04-01  0:47     ` [PATCH] tty: Fix merge of "tty: Refactor tty_open()" Peter Hurley
2016-04-01  3:49     ` linux-next: manual merge of the tty tree with the tty.current tree Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).