Re: [systemd-devel] [PATCH] udev: Restore udevadm settle timeout

2015-05-07 Thread Harald Hoyer 
On 20.04.2015 10:33, David Herrmann wrote:
 Hi
 
 On Sat, Apr 11, 2015 at 9:38 PM, Nir Soffer nir...@gmail.com wrote:
 On Sat, Apr 11, 2015 at 1:36 PM, David Herrmann dh.herrm...@gmail.com 
 wrote:
 @@ -139,6 +142,9 @@ static int adm_settle(struct udev *udev, int argc, 
 char *argv[]) {
  break;
  }

 +if (now(CLOCK_MONOTONIC) = deadline)
 +break;
 +

 Previous udevadm allowed timeout=0 to disable this. I added the condition.

 Hi David,

 I think the handling of timeout=0 is incorrect now. The manual says:

 A value of 0 will check if the queue is empty and always return
 immediately.

 In udev-147 (used on rhel6), this was the behavior. If timeout was 0,
 is_timeout was set and settle was returning with rc=1.

 This behavior changed in:

 http://git.kernel.org/cgit/linux/hotplug/udev.git/commit/?id=ead7c62ab7641e150c6d668f939c102a6771ce60

 After this commit, zero timeout results in unlimited wait. Since this
 patch did not
 change the manual or the online help, and the commit message says:
 udevadm: settle - kill alarm(), I guess this was unintended change.

 I don't see the use case for disabling the timeout, so it seems that
 we should fix
 this, restoring the behavior before this commit.

 What do you think?
 
 Ok, this is on me, sorry for that. I tried to keep the behavior from
 before the code-removal. I wasn't aware that this was not how it is
 documented.
 
 I'm actually not sure whether that was an intended change. It does not
 look like it was, indeed. Maybe Kay or Tom know more.. I have no idea
 whether timeout=0 is used in the wild.
 


Oh, dracut makes use of udevadm settle --timeout=0 all the time
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] udev: Restore udevadm settle timeout

2015-05-07 Thread David Herrmann 
Hi

On Thu, May 7, 2015 at 10:53 AM, Harald Hoyer harald.ho...@gmail.com wrote:
 On 20.04.2015 10:33, David Herrmann wrote:
 Hi

 On Sat, Apr 11, 2015 at 9:38 PM, Nir Soffer nir...@gmail.com wrote:
 On Sat, Apr 11, 2015 at 1:36 PM, David Herrmann dh.herrm...@gmail.com 
 wrote:
 @@ -139,6 +142,9 @@ static int adm_settle(struct udev *udev, int argc, 
 char *argv[]) {
  break;
  }

 +if (now(CLOCK_MONOTONIC) = deadline)
 +break;
 +

 Previous udevadm allowed timeout=0 to disable this. I added the condition.

 Hi David,

 I think the handling of timeout=0 is incorrect now. The manual says:

 A value of 0 will check if the queue is empty and always return
 immediately.

 In udev-147 (used on rhel6), this was the behavior. If timeout was 0,
 is_timeout was set and settle was returning with rc=1.

 This behavior changed in:

 http://git.kernel.org/cgit/linux/hotplug/udev.git/commit/?id=ead7c62ab7641e150c6d668f939c102a6771ce60

 After this commit, zero timeout results in unlimited wait. Since this
 patch did not
 change the manual or the online help, and the commit message says:
 udevadm: settle - kill alarm(), I guess this was unintended change.

 I don't see the use case for disabling the timeout, so it seems that
 we should fix
 this, restoring the behavior before this commit.

 What do you think?

 Ok, this is on me, sorry for that. I tried to keep the behavior from
 before the code-removal. I wasn't aware that this was not how it is
 documented.

 I'm actually not sure whether that was an intended change. It does not
 look like it was, indeed. Maybe Kay or Tom know more.. I have no idea
 whether timeout=0 is used in the wild.



 Oh, dracut makes use of udevadm settle --timeout=0 all the time

--timeout was ignored for udevadm-settle since 213, effectively
running with an infinite timeout under all circumstances. This was a
regression of:
udev: remove seqnum API and all assumptions about seqnums (commit 9ea28c55)

The 3 fixes to make --timeout work properly again, are:
udev: restore udevadm settle timeout (commit 0736455b)
udev: settle should return immediately when timeout is 0 (commit bf23b9f8)
udev: Fix ping timeout when settle timeout is 0 (commit 7375b3c4)

All are queued up for 219.

Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] udev: Restore udevadm settle timeout

2015-05-06 Thread David Herrmann 
Hi

On Wed, May 6, 2015 at 4:31 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Mon, Apr 20, 2015 at 10:33:48AM +0200, David Herrmann wrote:
 Hi

 On Sat, Apr 11, 2015 at 9:38 PM, Nir Soffer nir...@gmail.com wrote:
  On Sat, Apr 11, 2015 at 1:36 PM, David Herrmann dh.herrm...@gmail.com 
  wrote:
   @@ -139,6 +142,9 @@ static int adm_settle(struct udev *udev, int argc, 
   char *argv[]) {
break;
}
  
   +if (now(CLOCK_MONOTONIC) = deadline)
   +break;
   +
 
  Previous udevadm allowed timeout=0 to disable this. I added the condition.
 
  Hi David,
 
  I think the handling of timeout=0 is incorrect now. The manual says:
 
  A value of 0 will check if the queue is empty and always return
  immediately.
 
  In udev-147 (used on rhel6), this was the behavior. If timeout was 0,
  is_timeout was set and settle was returning with rc=1.
 
  This behavior changed in:
 
  http://git.kernel.org/cgit/linux/hotplug/udev.git/commit/?id=ead7c62ab7641e150c6d668f939c102a6771ce60
 
  After this commit, zero timeout results in unlimited wait. Since this
  patch did not
  change the manual or the online help, and the commit message says:
  udevadm: settle - kill alarm(), I guess this was unintended change.
 
  I don't see the use case for disabling the timeout, so it seems that
  we should fix
  this, restoring the behavior before this commit.
 
  What do you think?

 Ok, this is on me, sorry for that. I tried to keep the behavior from
 before the code-removal. I wasn't aware that this was not how it is
 documented.

 I'm actually not sure whether that was an intended change. It does not
 look like it was, indeed. Maybe Kay or Tom know more.. I have no idea
 whether timeout=0 is used in the wild.

 I'll stall your further patches until we've decided on this.
 What's the status here?

This is already fixed in -git. Original behavior is restored.

Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] udev: Restore udevadm settle timeout

2015-05-05 Thread Zbigniew Jędrzejewski-Szmek 
On Mon, Apr 20, 2015 at 10:33:48AM +0200, David Herrmann wrote:
 Hi
 
 On Sat, Apr 11, 2015 at 9:38 PM, Nir Soffer nir...@gmail.com wrote:
  On Sat, Apr 11, 2015 at 1:36 PM, David Herrmann dh.herrm...@gmail.com 
  wrote:
   @@ -139,6 +142,9 @@ static int adm_settle(struct udev *udev, int argc, 
   char *argv[]) {
break;
}
  
   +if (now(CLOCK_MONOTONIC) = deadline)
   +break;
   +
 
  Previous udevadm allowed timeout=0 to disable this. I added the condition.
 
  Hi David,
 
  I think the handling of timeout=0 is incorrect now. The manual says:
 
  A value of 0 will check if the queue is empty and always return
  immediately.
 
  In udev-147 (used on rhel6), this was the behavior. If timeout was 0,
  is_timeout was set and settle was returning with rc=1.
 
  This behavior changed in:
 
  http://git.kernel.org/cgit/linux/hotplug/udev.git/commit/?id=ead7c62ab7641e150c6d668f939c102a6771ce60
 
  After this commit, zero timeout results in unlimited wait. Since this
  patch did not
  change the manual or the online help, and the commit message says:
  udevadm: settle - kill alarm(), I guess this was unintended change.
 
  I don't see the use case for disabling the timeout, so it seems that
  we should fix
  this, restoring the behavior before this commit.
 
  What do you think?
 
 Ok, this is on me, sorry for that. I tried to keep the behavior from
 before the code-removal. I wasn't aware that this was not how it is
 documented.
 
 I'm actually not sure whether that was an intended change. It does not
 look like it was, indeed. Maybe Kay or Tom know more.. I have no idea
 whether timeout=0 is used in the wild.
 
 I'll stall your further patches until we've decided on this.
What's the status here?

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] udev: Restore udevadm settle timeout

2015-04-20 Thread David Herrmann 
Hi

On Sat, Apr 11, 2015 at 9:38 PM, Nir Soffer nir...@gmail.com wrote:
 On Sat, Apr 11, 2015 at 1:36 PM, David Herrmann dh.herrm...@gmail.com wrote:
  @@ -139,6 +142,9 @@ static int adm_settle(struct udev *udev, int argc, 
  char *argv[]) {
   break;
   }
 
  +if (now(CLOCK_MONOTONIC) = deadline)
  +break;
  +

 Previous udevadm allowed timeout=0 to disable this. I added the condition.

 Hi David,

 I think the handling of timeout=0 is incorrect now. The manual says:

 A value of 0 will check if the queue is empty and always return
 immediately.

 In udev-147 (used on rhel6), this was the behavior. If timeout was 0,
 is_timeout was set and settle was returning with rc=1.

 This behavior changed in:

 http://git.kernel.org/cgit/linux/hotplug/udev.git/commit/?id=ead7c62ab7641e150c6d668f939c102a6771ce60

 After this commit, zero timeout results in unlimited wait. Since this
 patch did not
 change the manual or the online help, and the commit message says:
 udevadm: settle - kill alarm(), I guess this was unintended change.

 I don't see the use case for disabling the timeout, so it seems that
 we should fix
 this, restoring the behavior before this commit.

 What do you think?

Ok, this is on me, sorry for that. I tried to keep the behavior from
before the code-removal. I wasn't aware that this was not how it is
documented.

I'm actually not sure whether that was an intended change. It does not
look like it was, indeed. Maybe Kay or Tom know more.. I have no idea
whether timeout=0 is used in the wild.

I'll stall your further patches until we've decided on this.

Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] udev: Restore udevadm settle timeout

2015-04-11 Thread Nir Soffer 
On Sat, Apr 11, 2015 at 1:36 PM, David Herrmann dh.herrm...@gmail.com wrote:
  @@ -139,6 +142,9 @@ static int adm_settle(struct udev *udev, int argc, char 
  *argv[]) {
   break;
   }
 
  +if (now(CLOCK_MONOTONIC) = deadline)
  +break;
  +

 Previous udevadm allowed timeout=0 to disable this. I added the condition.

Hi David,

I think the handling of timeout=0 is incorrect now. The manual says:

A value of 0 will check if the queue is empty and always return
immediately.

In udev-147 (used on rhel6), this was the behavior. If timeout was 0,
is_timeout was set and settle was returning with rc=1.

This behavior changed in:

http://git.kernel.org/cgit/linux/hotplug/udev.git/commit/?id=ead7c62ab7641e150c6d668f939c102a6771ce60

After this commit, zero timeout results in unlimited wait. Since this
patch did not
change the manual or the online help, and the commit message says:
udevadm: settle - kill alarm(), I guess this was unintended change.

I don't see the use case for disabling the timeout, so it seems that
we should fix
this, restoring the behavior before this commit.

What do you think?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] udev: Restore udevadm settle timeout

2015-04-11 Thread David Herrmann 
Hi

On Wed, Apr 8, 2015 at 3:04 AM, Nir Soffer nir...@gmail.com wrote:
 Commit 9ea28c55a2 (udev: remove seqnum API and all assumptions about
 seqnums) introduced a regresion, ignoring the timeout option when
 waiting until the event queue is empty.

 Previously, if the udev event queue was not empty when the timeout was
 expired, udevadm settle was returning with exit code 1.  To check if the
 queue is empty, you could invoke udevadm settle with timeout=0. This
 patch restores the previous behavior.
 ---
  src/udev/udevadm-settle.c | 6 ++
  1 file changed, 6 insertions(+)

 diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
 index 0d3025e..715d2e7 100644
 --- a/src/udev/udevadm-settle.c
 +++ b/src/udev/udevadm-settle.c
 @@ -49,6 +49,7 @@ static int adm_settle(struct udev *udev, int argc, char 
 *argv[]) {
  { quiet,  no_argument,   NULL, 'q' }, /* 
 removed */
  {}
  };
 +usec_t deadline = 0;

This assignment is not required, dropped.

  const char *exists = NULL;
  unsigned int timeout = 120;
  struct pollfd pfd[1] = { {.fd = -1}, };
 @@ -98,6 +99,8 @@ static int adm_settle(struct udev *udev, int argc, char 
 *argv[]) {
  return EXIT_FAILURE;
  }

 +deadline = now(CLOCK_MONOTONIC) + timeout * USEC_PER_SEC;
 +
  /* guarantee that the udev daemon isn't pre-processing */
  if (getuid() == 0) {
  struct udev_ctrl *uctrl;
 @@ -139,6 +142,9 @@ static int adm_settle(struct udev *udev, int argc, char 
 *argv[]) {
  break;
  }

 +if (now(CLOCK_MONOTONIC) = deadline)
 +break;
 +

Previous udevadm allowed timeout=0 to disable this. I added the condition.

Applied!

Thanks
David

  /* wake up when queue is empty */
  if (poll(pfd, 1, MSEC_PER_SEC)  0  pfd[0].revents  
 POLLIN)
  udev_queue_flush(queue);
 --
 1.9.3

 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] udev: Restore udevadm settle timeout

2015-04-07 Thread Nir Soffer 
Commit 9ea28c55a2 (udev: remove seqnum API and all assumptions about
seqnums) introduced a regresion, ignoring the timeout option when
waiting until the event queue is empty.

Previously, if the udev event queue was not empty when the timeout was
expired, udevadm settle was returning with exit code 1.  To check if the
queue is empty, you could invoke udevadm settle with timeout=0. This
patch restores the previous behavior.
---
 src/udev/udevadm-settle.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index 0d3025e..715d2e7 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -49,6 +49,7 @@ static int adm_settle(struct udev *udev, int argc, char 
*argv[]) {
 { quiet,  no_argument,   NULL, 'q' }, /* removed 
*/
 {}
 };
+usec_t deadline = 0;
 const char *exists = NULL;
 unsigned int timeout = 120;
 struct pollfd pfd[1] = { {.fd = -1}, };
@@ -98,6 +99,8 @@ static int adm_settle(struct udev *udev, int argc, char 
*argv[]) {
 return EXIT_FAILURE;
 }
 
+deadline = now(CLOCK_MONOTONIC) + timeout * USEC_PER_SEC;
+
 /* guarantee that the udev daemon isn't pre-processing */
 if (getuid() == 0) {
 struct udev_ctrl *uctrl;
@@ -139,6 +142,9 @@ static int adm_settle(struct udev *udev, int argc, char 
*argv[]) {
 break;
 }
 
+if (now(CLOCK_MONOTONIC) = deadline)
+break;
+
 /* wake up when queue is empty */
 if (poll(pfd, 1, MSEC_PER_SEC)  0  pfd[0].revents  POLLIN)
 udev_queue_flush(queue);
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel