netdev - Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 3 Jul 2013 12:26:33 -0700
From:	Roland Dreier <roland@...nel.org>
To:	Or Gerlitz <ogerlitz@...lanox.com>
Cc:	Or Gerlitz <or.gerlitz@...il.com>,
	Eli Cohen <eli@....mellanox.co.il>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	Eli Cohen <eli@...lanox.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Jack Morgenstein <jackm@....mellanox.co.il>
Subject: Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

On Wed, Jul 3, 2013 at 9:41 AM, Or Gerlitz <ogerlitz@...lanox.com> wrote:
> Jack looked on this comment/code and he says that the active flag is used
> to prevent re-scheduling the timer from inside the timer handling routine.
>
> In the kernel, the comment header in the source file for del_timer_sync
> explicitly states that re-scheduling the timer must be prevented,
> or the sync is useless:Callers must prevent restarting of the timer,
> otherwise
> this function is meaningless
>
> So we believe that code should remain.

Look at the actual timer code.  del_timer_sync() won't work if
something unrelated re-adds the timer, but it will work if the timer
itself is what re-adds itself.

Documentation/DocBook/kernel-locking.tmpl says:

      Another common problem is deleting timers which restart
      themselves (by calling <function>add_timer()</function> at the end
      of their timer function).  Because this is a fairly common case
      which is prone to races, you should use
<function>del_timer_sync()</function>
      (<filename class="headerfile">include/linux/timer.h</filename>)
      to handle this case.  It returns the number of times the timer
      had to be deleted before we finally stopped it from adding itself back
      in.

which pretty clearly says that del_timer_sync() will work in this case.

Or look at the code using it in arch/sparc/kernel/led.c for example
(just one of the first hits in my grep, there are many other
examples).

Not a big deal but I'm pretty sure the flag isn't needed.

 - R.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html