There is a timer_list race condition if a delay work is queued twice,
this usually won't happen unless someone reinitializes the task before performing the enqueue operation,likeï¼?
https://github.com/torvalds/linux/blob/master/drivers/devfreq/devfreq.c#L487
A warning message will help developers identify this irregular usage.
Signed-off-by: ZaiYang Huang <[email protected]>
---
include/linux/workqueue.h | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 24b1e5070f4d..54102ed794e5 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -266,6 +266,22 @@ static inline void destroy_delayed_work_on_stack(struct delayed_work *work) { }
static inline unsigned int work_static(struct work_struct *work) { return 0; }
#endif
+/**
+ * work_pending - Find out whether a work item is currently pending
+ * @work: The work item in question
+ */
+#define work_pending(work) \
+ test_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))
+
+/**
+ * delayed_work_pending - Find out whether a delayable work item is currently
+ * pending
+ * @w: The work item in question
+ */
+#define delayed_work_pending(w) \
+ work_pending(&(w)->work)
+
+
/*
* initialize all of a work item in one go
*
@@ -310,6 +326,7 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
#define __INIT_DELAYED_WORK(_work, _func, _tflags) \
do { \
+ WARN_ON(delayed_work_pending(_work)); \
INIT_WORK(&(_work)->work, (_func)); \
__init_timer(&(_work)->timer, \
delayed_work_timer_fn, \
@@ -318,6 +335,7 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
#define __INIT_DELAYED_WORK_ONSTACK(_work, _func, _tflags) \
do { \
+ WARN_ON(delayed_work_pending(_work)); \
INIT_WORK_ONSTACK(&(_work)->work, (_func)); \
__init_timer_on_stack(&(_work)->timer, \
delayed_work_timer_fn, \
@@ -342,21 +360,6 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
#define INIT_RCU_WORK_ONSTACK(_work, _func) \
INIT_WORK_ONSTACK(&(_work)->work, (_func))
-/**
- * work_pending - Find out whether a work item is currently pending
- * @work: The work item in question
- */
-#define work_pending(work) \
- test_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))
-
-/**
- * delayed_work_pending - Find out whether a delayable work item is currently
- * pending
- * @w: The work item in question
- */
-#define delayed_work_pending(w) \
- work_pending(&(w)->work)
-
/*
* Workqueue flags and constants. For details, please refer to
* Documentation/core-api/workqueue.rst.
--
2.17.1
________________________________
OPPO
±¾µç×ÓÓʼþ¼°Æ丽¼þº¬ÓÐOPPO¹«Ë¾µÄ±£ÃÜÐÅÏ¢£¬½öÏÞÓÚÓʼþÖ¸Ã÷µÄÊÕ¼þÈË£¨°üº¬¸öÈ˼°Èº×飩ʹÓ᣽ûÖ¹ÈκÎÈËÔÚδ¾ÊÚȨµÄÇé¿öÏÂÒÔÈκÎÐÎʽʹÓá£Èç¹ûÄú´íÊÕÁ˱¾Óʼþ£¬ÇÐÎð´«²¥¡¢·Ö·¢¡¢¸´ÖÆ¡¢Ó¡Ë¢»òʹÓñ¾ÓʼþÖ®Èκβ¿·Ö»òÆäËùÔØÖ®ÈκÎÄÚÈÝ£¬²¢ÇëÁ¢¼´ÒÔµç×ÓÓʼþ֪ͨ·¢¼þÈ˲¢É¾³ý±¾Óʼþ¼°Æ丽¼þ¡£
ÍøÂçͨѶ¹ÌÓÐȱÏÝ¿ÉÄܵ¼ÖÂÓʼþ±»½ØÁô¡¢Ð޸ġ¢¶ªÊ§¡¢ÆÆ»µ»ò°üº¬¼ÆËã»ú²¡¶¾µÈ²»°²È«Çé¿ö£¬OPPO¶Ô´ËÀà´íÎó»òÒÅ©¶øÒýÖÂÖ®ÈκÎËðʧ¸Å²»³Ðµ£ÔðÈβ¢±£ÁôÓë±¾ÓʼþÏà¹ØÖ®Ò»ÇÐȨÀû¡£
³ý·ÇÃ÷ȷ˵Ã÷£¬±¾Óʼþ¼°Æ丽¼þÎÞÒâ×÷ΪÔÚÈκιú¼Ò»òµØÇøÖ®ÒªÔ¼¡¢ÕÐÀ¿»ò³Ðŵ£¬ÒàÎÞÒâ×÷ΪÈκν»Ò×»òºÏ֮ͬÕýʽȷÈÏ¡£ ·¢¼þÈË¡¢ÆäËùÊô»ú¹¹»òËùÊô»ú¹¹Ö®¹ØÁª»ú¹¹»òÈκÎÉÏÊö»ú¹¹Ö®¹É¶«¡¢¶Ê¡¢¸ß¼¶¹ÜÀíÈËÔ±¡¢Ô±¹¤»òÆäËûÈκÎÈË£¨ÒÔϳơ°·¢¼þÈË¡±»ò¡°OPPO¡±£©²»Òò±¾ÓʼþÖ®ÎóËͶø·ÅÆúÆäËùÏíÖ®ÈκÎȨÀû£¬Ò಻¶ÔÒò¹ÊÒâ»ò¹ýʧʹÓøõÈÐÅÏ¢¶øÒý·¢»ò¿ÉÄÜÒý·¢µÄËðʧ³Ðµ£ÈκÎÔðÈΡ£
ÎÄ»¯²îÒìÅû¶£ºÒòÈ«ÇòÎÄ»¯²îÒìÓ°Ï죬µ¥´¿ÒÔYES\OK»òÆäËû¼òµ¥´Ê»ãµÄ»Ø¸´²¢²»¹¹³É·¢¼þÈ˶ÔÈκν»Ò×»òºÏ֮ͬÕýʽȷÈÏ»ò½ÓÊÜ£¬ÇëÓë·¢¼þÈËÔÙ´ÎÈ·ÈÏÒÔ»ñµÃÃ÷È·ÊéÃæÒâ¼û¡£·¢¼þÈ˲»¶ÔÈκÎÊÜÎÄ»¯²îÒìÓ°Ïì¶øµ¼Ö¹ÊÒâ»ò´íÎóʹÓøõÈÐÅÏ¢ËùÔì³ÉµÄÈκÎÖ±½Ó»ò¼ä½ÓË𺦳е£ÔðÈΡ£
This e-mail and its attachments contain confidential information from OPPO, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you are not the intended recipient, please do not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.
Electronic communications may contain computer viruses or other defects inherently, may not be accurately and/or timely transmitted to other systems, or may be intercepted, modified ,delayed, deleted or interfered. OPPO shall not be liable for any damages that arise or may arise from such matter and reserves all rights in connection with the email.
Unless expressly stated, this e-mail and its attachments are provided without any warranty, acceptance or promise of any kind in any country or region, nor constitute a formal confirmation or acceptance of any transaction or contract. The sender, together with its affiliates or any shareholder, director, officer, employee or any other person of any such institution (hereinafter referred to as "sender" or "OPPO") does not waive any rights and shall not be liable for any damages that arise or may arise from the intentional or negligent use of such information.
Cultural Differences Disclosure: Due to global cultural differences, any reply with only YES\OK or other simple words does not constitute any confirmation or acceptance of any transaction or contract, please confirm with the sender again to ensure clear opinion in written form. The sender shall not be responsible for any direct or indirect damages resulting from the intentional or misuse of such information.
________________________________
OPPO
本电子邮件及其附件含有OPPO公司的保密信息，仅限于邮件指明的收件人（包含个人及群组）使用。禁止任何人在未经授权的情况下以任何形式使用。如果您错收了本邮件，切勿传播、分发、复制、印刷或使用本邮件之任何部分或其所载之任何内容，并请立即以电子邮件通知发件人并删除本邮件及其附件。
网络通讯固有缺陷可能导致邮件被截留、修改、丢失、破坏或包含计算机病毒等不安全情况，OPPO对此类错误或遗漏而引致之任何损失概不承担责任并保留与本邮件相关之一切权利。
除非明确说明，本邮件及其附件无意作为在任何国家或地区之要约、招揽或承诺，亦无意作为任何交易或合同之正式确认。 发件人、其所属机构或所属机构之关联机构或任何上述机构之股东、董事、高级管理人员、员工或其他任何人（以下称“发件人”或“OPPO”）不因本邮件之误送而放弃其所享之任何权利，亦不对因故意或过失使用该等信息而引发或可能引发的损失承担任何责任。
文化差异披露：因全球文化差异影响，单纯以YES\OK或其他简单词汇的回复并不构成发件人对任何交易或合同之正式确认或接受，请与发件人再次确认以获得明确书面意见。发件人不对任何受文化差异影响而导致故意或错误使用该等信息所造成的任何直接或间接损害承担责任。
This e-mail and its attachments contain confidential information from OPPO, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you are not the intended recipient, please do not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.
Electronic communications may contain computer viruses or other defects inherently, may not be accurately and/or timely transmitted to other systems, or may be intercepted, modified ,delayed, deleted or interfered. OPPO shall not be liable for any damages that arise or may arise from such matter and reserves all rights in connection with the email.
Unless expressly stated, this e-mail and its attachments are provided without any warranty, acceptance or promise of any kind in any country or region, nor constitute a formal confirmation or acceptance of any transaction or contract. The sender, together with its affiliates or any shareholder, director, officer, employee or any other person of any such institution (hereinafter referred to as "sender" or "OPPO") does not waive any rights and shall not be liable for any damages that arise or may arise from the intentional or negligent use of such information.
Cultural Differences Disclosure: Due to global cultural differences, any reply with only YES\OK or other simple words does not constitute any confirmation or acceptance of any transaction or contract, please confirm with the sender again to ensure clear opinion in written form. The sender shall not be responsible for any direct or indirect damages resulting from the intentional or misuse of such information.
On 11/15/2023 5:04 PM, huangzaiyang wrote:
> There is a timer_list race condition if a delay work is queued twice,
> this usually won't happen unless someone reinitializes the task before performing the enqueue operation,likeï¼?
> https://github.com/torvalds/linux/blob/master/drivers/devfreq/devfreq.c#L487
> A warning message will help developers identify this irregular usage.
>
> Signed-off-by: ZaiYang Huang <[email protected]>
I like the idea to have this for developers.
Acked-by: Mukesh Ojha <[email protected]>
> ---
> include/linux/workqueue.h | 33 ++++++++++++++++++---------------
> 1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index 24b1e5070f4d..54102ed794e5 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -266,6 +266,22 @@ static inline void destroy_delayed_work_on_stack(struct delayed_work *work) { }
> static inline unsigned int work_static(struct work_struct *work) { return 0; }
> #endif
>
> +/**
> + * work_pending - Find out whether a work item is currently pending
> + * @work: The work item in question
> + */
> +#define work_pending(work) \
> + test_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))
> +
> +/**
> + * delayed_work_pending - Find out whether a delayable work item is currently
> + * pending
> + * @w: The work item in question
> + */
> +#define delayed_work_pending(w) \
> + work_pending(&(w)->work)
> +
> +
> /*
> * initialize all of a work item in one go
> *
> @@ -310,6 +326,7 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
>
> #define __INIT_DELAYED_WORK(_work, _func, _tflags) \
> do { \
> + WARN_ON(delayed_work_pending(_work)); \
> INIT_WORK(&(_work)->work, (_func)); \
> __init_timer(&(_work)->timer, \
> delayed_work_timer_fn, \
> @@ -318,6 +335,7 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
>
> #define __INIT_DELAYED_WORK_ONSTACK(_work, _func, _tflags) \
> do { \
> + WARN_ON(delayed_work_pending(_work));
However, I don't think it will be applicable for _ONSTACK variable as it
is on stack.
\
> INIT_WORK_ONSTACK(&(_work)->work, (_func)); \
> __init_timer_on_stack(&(_work)->timer, \
> delayed_work_timer_fn, \
> @@ -342,21 +360,6 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
> #define INIT_RCU_WORK_ONSTACK(_work, _func) \
> INIT_WORK_ONSTACK(&(_work)->work, (_func))
>
> -/**
> - * work_pending - Find out whether a work item is currently pending
> - * @work: The work item in question
> - */
> -#define work_pending(work) \
> - test_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))
> -
> -/**
> - * delayed_work_pending - Find out whether a delayable work item is currently
> - * pending
> - * @w: The work item in question
> - */
> -#define delayed_work_pending(w) \
> - work_pending(&(w)->work)
> -
> /*
> * Workqueue flags and constants. For details, please refer to
> * Documentation/core-api/workqueue.rst.
> --
> 2.17.1
Please remove this ..
-Mukesh
>
> ________________________________
> OPPO
>
> ±¾µç×ÓÓʼþ¼°Æ丽¼þº¬ÓÐOPPO¹«Ë¾µÄ±£ÃÜÐÅÏ¢£¬½öÏÞÓÚÓʼþÖ¸Ã÷µÄÊÕ¼þÈË£¨°üº¬¸öÈ˼°Èº×飩ʹÓ᣽ûÖ¹ÈκÎÈËÔÚδ¾ÊÚȨµÄÇé¿öÏÂÒÔÈκÎÐÎʽʹÓá£Èç¹ûÄú´íÊÕÁ˱¾Óʼþ£¬ÇÐÎð´«²¥¡¢·Ö·¢¡¢¸´ÖÆ¡¢Ó¡Ë¢»òʹÓñ¾ÓʼþÖ®Èκβ¿·Ö»òÆäËùÔØÖ®ÈκÎÄÚÈÝ£¬²¢ÇëÁ¢¼´ÒÔµç×ÓÓʼþ֪ͨ·¢¼þÈ˲¢É¾³ý±¾Óʼþ¼°Æ丽¼þ¡£
> ÍøÂçͨѶ¹ÌÓÐȱÏÝ¿ÉÄܵ¼ÖÂÓʼþ±»½ØÁô¡¢Ð޸ġ¢¶ªÊ§¡¢ÆÆ»µ»ò°üº¬¼ÆËã»ú²¡¶¾µÈ²»°²È«Çé¿ö£¬OPPO¶Ô´ËÀà´íÎó»òÒÅ©¶øÒýÖÂÖ®ÈκÎËðʧ¸Å²»³Ðµ£ÔðÈβ¢±£ÁôÓë±¾ÓʼþÏà¹ØÖ®Ò»ÇÐȨÀû¡£
> ³ý·ÇÃ÷ȷ˵Ã÷£¬±¾Óʼþ¼°Æ丽¼þÎÞÒâ×÷ΪÔÚÈκιú¼Ò»òµØÇøÖ®ÒªÔ¼¡¢ÕÐÀ¿»ò³Ðŵ£¬ÒàÎÞÒâ×÷ΪÈκν»Ò×»òºÏ֮ͬÕýʽȷÈÏ¡£ ·¢¼þÈË¡¢ÆäËùÊô»ú¹¹»òËùÊô»ú¹¹Ö®¹ØÁª»ú¹¹»òÈκÎÉÏÊö»ú¹¹Ö®¹É¶«¡¢¶Ê¡¢¸ß¼¶¹ÜÀíÈËÔ±¡¢Ô±¹¤»òÆäËûÈκÎÈË£¨ÒÔϳơ°·¢¼þÈË¡±»ò¡°OPPO¡±£©²»Òò±¾ÓʼþÖ®ÎóËͶø·ÅÆúÆäËùÏíÖ®ÈκÎȨÀû£¬Ò಻¶ÔÒò¹ÊÒâ»ò¹ýʧʹÓøõÈÐÅÏ¢¶øÒý·¢»ò¿ÉÄÜÒý·¢µÄËðʧ³Ðµ£ÈκÎÔðÈΡ£
> ÎÄ»¯²îÒìÅû¶£ºÒòÈ«ÇòÎÄ»¯²îÒìÓ°Ï죬µ¥´¿ÒÔYES\OK»òÆäËû¼òµ¥´Ê»ãµÄ»Ø¸´²¢²»¹¹³É·¢¼þÈ˶ÔÈκν»Ò×»òºÏ֮ͬÕýʽȷÈÏ»ò½ÓÊÜ£¬ÇëÓë·¢¼þÈËÔÙ´ÎÈ·ÈÏÒÔ»ñµÃÃ÷È·ÊéÃæÒâ¼û¡£·¢¼þÈ˲»¶ÔÈκÎÊÜÎÄ»¯²îÒìÓ°Ïì¶øµ¼Ö¹ÊÒâ»ò´íÎóʹÓøõÈÐÅÏ¢ËùÔì³ÉµÄÈκÎÖ±½Ó»ò¼ä½ÓË𺦳е£ÔðÈΡ£
> This e-mail and its attachments contain confidential information from OPPO, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you are not the intended recipient, please do not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.
> Electronic communications may contain computer viruses or other defects inherently, may not be accurately and/or timely transmitted to other systems, or may be intercepted, modified ,delayed, deleted or interfered. OPPO shall not be liable for any damages that arise or may arise from such matter and reserves all rights in connection with the email.
> Unless expressly stated, this e-mail and its attachments are provided without any warranty, acceptance or promise of any kind in any country or region, nor constitute a formal confirmation or acceptance of any transaction or contract. The sender, together with its affiliates or any shareholder, director, officer, employee or any other person of any such institution (hereinafter referred to as "sender" or "OPPO") does not waive any rights and shall not be liable for any damages that arise or may arise from the intentional or negligent use of such information.
> Cultural Differences Disclosure: Due to global cultural differences, any reply with only YES\OK or other simple words does not constitute any confirmation or acceptance of any transaction or contract, please confirm with the sender again to ensure clear opinion in written form. The sender shall not be responsible for any direct or indirect damages resulting from the intentional or misuse of such information.
> ________________________________
> OPPO
>
> 本电子邮件及其附件含有OPPO公司的保密信息，仅限于邮件指明的收件人（包含个人及群组）使用。禁止任何人在未经授权的情况下以任何形式使用。如果您错收了本邮件，切勿传播、分发、复制、印刷或使用本邮件之任何部分或其所载之任何内容，并请立即以电子邮件通知发件人并删除本邮件及其附件。
> 网络通讯固有缺陷可能导致邮件被截留、修改、丢失、破坏或包含计算机病毒等不安全情况，OPPO对此类错误或遗漏而引致之任何损失概不承担责任并保留与本邮件相关之一切权利。
> 除非明确说明，本邮件及其附件无意作为在任何国家或地区之要约、招揽或承诺，亦无意作为任何交易或合同之正式确认。 发件人、其所属机构或所属机构之关联机构或任何上述机构之股东、董事、高级管理人员、员工或其他任何人（以下称“发件人”或“OPPO”）不因本邮件之误送而放弃其所享之任何权利，亦不对因故意或过失使用该等信息而引发或可能引发的损失承担任何责任。
> 文化差异披露：因全球文化差异影响，单纯以YES\OK或其他简单词汇的回复并不构成发件人对任何交易或合同之正式确认或接受，请与发件人再次确认以获得明确书面意见。发件人不对任何受文化差异影响而导致故意或错误使用该等信息所造成的任何直接或间接损害承担责任。
> This e-mail and its attachments contain confidential information from OPPO, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you are not the intended recipient, please do not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.
> Electronic communications may contain computer viruses or other defects inherently, may not be accurately and/or timely transmitted to other systems, or may be intercepted, modified ,delayed, deleted or interfered. OPPO shall not be liable for any damages that arise or may arise from such matter and reserves all rights in connection with the email.
> Unless expressly stated, this e-mail and its attachments are provided without any warranty, acceptance or promise of any kind in any country or region, nor constitute a formal confirmation or acceptance of any transaction or contract. The sender, together with its affiliates or any shareholder, director, officer, employee or any other person of any such institution (hereinafter referred to as "sender" or "OPPO") does not waive any rights and shall not be liable for any damages that arise or may arise from the intentional or negligent use of such information.
> Cultural Differences Disclosure: Due to global cultural differences, any reply with only YES\OK or other simple words does not constitute any confirmation or acceptance of any transaction or contract, please confirm with the sender again to ensure clear opinion in written form. The sender shall not be responsible for any direct or indirect damages resulting from the intentional or misuse of such information.
On 2023-11-15 19:34:27+0800, huangzaiyang wrote:
> There is a timer_list race condition if a delay work is queued twice,
> this usually won't happen unless someone reinitializes the task before performing the enqueue operation,likeï¼?
> https://github.com/torvalds/linux/blob/master/drivers/devfreq/devfreq.c#L487
> A warning message will help developers identify this irregular usage.
>
> Signed-off-by: ZaiYang Huang <[email protected]>
> ---
> include/linux/workqueue.h | 33 ++++++++++++++++++---------------
> 1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index 24b1e5070f4d..54102ed794e5 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -266,6 +266,22 @@ static inline void destroy_delayed_work_on_stack(struct delayed_work *work) { }
> static inline unsigned int work_static(struct work_struct *work) { return 0; }
> #endif
>
> +/**
> + * work_pending - Find out whether a work item is currently pending
> + * @work: The work item in question
> + */
> +#define work_pending(work) \
> + test_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))
> +
> +/**
> + * delayed_work_pending - Find out whether a delayable work item is currently
> + * pending
> + * @w: The work item in question
> + */
> +#define delayed_work_pending(w) \
> + work_pending(&(w)->work)
> +
> +
> /*
> * initialize all of a work item in one go
> *
> @@ -310,6 +326,7 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
>
> #define __INIT_DELAYED_WORK(_work, _func, _tflags) \
> do { \
> + WARN_ON(delayed_work_pending(_work)); \
How does this work when the data _work points to is not yet initialized?
Reading uninitialized data is UB and may spuriously trigger the warning.
> INIT_WORK(&(_work)->work, (_func)); \
> __init_timer(&(_work)->timer, \
> delayed_work_timer_fn, \
> @@ -318,6 +335,7 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
[..]
> --
> 2.17.1
>
> ________________________________
> OPPO
The gunk at the end of the mail will prevent your patch from being
considered at all.
>
> ±¾µç×ÓÓʼþ¼°Æ丽¼þº¬ÓÐOPPO¹«Ë¾µÄ±£ÃÜÐÅÏ¢£¬½öÏÞÓÚÓʼþÖ¸Ã÷µÄÊÕ¼þÈË£¨°üº¬¸öÈ˼°Èº×飩ʹÓ᣽ûÖ¹ÈκÎÈËÔÚδ¾ÊÚȨµÄÇé¿öÏÂÒÔÈκÎÐÎʽʹÓá£Èç¹ûÄú´íÊÕÁ˱¾Óʼþ£¬ÇÐÎð´«²¥¡¢·Ö·¢¡¢¸´ÖÆ¡¢Ó¡Ë¢»òʹÓñ¾ÓʼþÖ®Èκβ¿·Ö»òÆäËùÔØÖ®ÈκÎÄÚÈÝ£¬²¢ÇëÁ¢¼´ÒÔµç×ÓÓʼþ֪ͨ·¢¼þÈ˲¢É¾³ý±¾Óʼþ¼°Æ丽¼þ¡£
[..]
On 2023-11-15 19:34:27+0800, huangzaiyang wrote:
> There is a timer_list race condition if a delay work is queued twice,
> this usually won't happen unless someone reinitializes the task before performing the enqueue operation,likeï¼?
> https://github.com/torvalds/linux/blob/master/drivers/devfreq/devfreq.
> c#L487 A warning message will help developers identify this irregular
> usage.
>
> Signed-off-by: ZaiYang Huang <[email protected]>
> ---
> include/linux/workqueue.h | 33 ++++++++++++++++++---------------
> 1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index 24b1e5070f4d..54102ed794e5 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -266,6 +266,22 @@ static inline void
> destroy_delayed_work_on_stack(struct delayed_work *work) { } static
> inline unsigned int work_static(struct work_struct *work) { return 0;
> } #endif
>
> +/**
> + * work_pending - Find out whether a work item is currently pending
> + * @work: The work item in question
> + */
> +#define work_pending(work) \
> + test_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))
> +
> +/**
> + * delayed_work_pending - Find out whether a delayable work item is
> +currently
> + * pending
> + * @w: The work item in question
> + */
> +#define delayed_work_pending(w) \
> + work_pending(&(w)->work)
> +
> +
> /*
> * initialize all of a work item in one go
> *
> @@ -310,6 +326,7 @@ static inline unsigned int work_static(struct
> work_struct *work) { return 0; }
>
> #define __INIT_DELAYED_WORK(_work, _func, _tflags) \
> do { \
> + WARN_ON(delayed_work_pending(_work)); \
How does this work when the data _work points to is not yet initialized?
Reading uninitialized data is UB and may spuriously trigger the warning.
--> The data member in work_struct is a variable of type atomic_long_t, and 0 is the initial value of the atomic variable, right?
> INIT_WORK(&(_work)->work, (_func)); \
> __init_timer(&(_work)->timer, \
> delayed_work_timer_fn, \
> @@ -318,6 +335,7 @@ static inline unsigned int work_static(struct
> work_struct *work) { return 0; }
[..]
> --
> 2.17.1
>
> ________________________________
> OPPO
The gunk at the end of the mail will prevent your patch from being considered at all.
>
> ±¾µç×ÓÓʼþ¼°Æ丽¼þº¬ÓÐOPPO¹«Ë¾µÄ±£ÃÜÐÅÏ¢£¬½öÏÞÓÚÓʼþÖ¸Ã÷µÄÊÕ¼þÈË£¨°üº¬
> ¸öÈ˼°Èº×飩ʹÓ᣽ûÖ¹ÈκÎÈËÔÚδ¾ÊÚȨµÄÇé¿öÏÂÒÔÈκÎÐÎʽʹÓá£Èç¹ûÄú´í
> ÊÕÁ˱¾Óʼþ£¬ÇÐÎð´«²¥¡¢·Ö·¢¡¢¸´ÖÆ¡¢Ó¡Ë¢»òʹÓñ¾ÓʼþÖ®Èκβ¿·Ö»òÆäËùÔØÖ®
> ÈκÎÄÚÈÝ£¬²¢ÇëÁ¢¼´ÒÔµç×ÓÓʼþ֪ͨ·¢¼þÈ˲¢É¾³ý±¾Óʼþ¼°Æ丽¼þ¡£
[..]
________________________________
OPPO
本电子邮件及其附件含有OPPO公司的保密信息，仅限于邮件指明的收件人（包含个人及群组）使用。禁止任何人在未经授权的情况下以任何形式使用。如果您错收了本邮件，切勿传播、分发、复制、印刷或使用本邮件之任何部分或其所载之任何内容，并请立即以电子邮件通知发件人并删除本邮件及其附件。
网络通讯固有缺陷可能导致邮件被截留、修改、丢失、破坏或包含计算机病毒等不安全情况，OPPO对此类错误或遗漏而引致之任何损失概不承担责任并保留与本邮件相关之一切权利。
除非明确说明，本邮件及其附件无意作为在任何国家或地区之要约、招揽或承诺，亦无意作为任何交易或合同之正式确认。 发件人、其所属机构或所属机构之关联机构或任何上述机构之股东、董事、高级管理人员、员工或其他任何人（以下称“发件人”或“OPPO”）不因本邮件之误送而放弃其所享之任何权利，亦不对因故意或过失使用该等信息而引发或可能引发的损失承担任何责任。
文化差异披露：因全球文化差异影响，单纯以YES\OK或其他简单词汇的回复并不构成发件人对任何交易或合同之正式确认或接受，请与发件人再次确认以获得明确书面意见。发件人不对任何受文化差异影响而导致故意或错误使用该等信息所造成的任何直接或间接损害承担责任。
This e-mail and its attachments contain confidential information from OPPO, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you are not the intended recipient, please do not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.
Electronic communications may contain computer viruses or other defects inherently, may not be accurately and/or timely transmitted to other systems, or may be intercepted, modified ,delayed, deleted or interfered. OPPO shall not be liable for any damages that arise or may arise from such matter and reserves all rights in connection with the email.
Unless expressly stated, this e-mail and its attachments are provided without any warranty, acceptance or promise of any kind in any country or region, nor constitute a formal confirmation or acceptance of any transaction or contract. The sender, together with its affiliates or any shareholder, director, officer, employee or any other person of any such institution (hereinafter referred to as "sender" or "OPPO") does not waive any rights and shall not be liable for any damages that arise or may arise from the intentional or negligent use of such information.
Cultural Differences Disclosure: Due to global cultural differences, any reply with only YES\OK or other simple words does not constitute any confirmation or acceptance of any transaction or contract, please confirm with the sender again to ensure clear opinion in written form. The sender shall not be responsible for any direct or indirect damages resulting from the intentional or misuse of such information.
On 2023-11-15 19:34:27+0800, huangzaiyang wrote:
> There is a timer_list race condition if a delay work is queued twice,
> this usually won't happen unless someone reinitializes the task before performing the enqueue operation,likeï¼?
> https://github.com/torvalds/linux/blob/master/drivers/devfreq/devfreq.
> c#L487 A warning message will help developers identify this irregular
> usage.
>
> Signed-off-by: ZaiYang Huang <[email protected]>
> ---
> include/linux/workqueue.h | 33 ++++++++++++++++++---------------
> 1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index 24b1e5070f4d..54102ed794e5 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -266,6 +266,22 @@ static inline void
> destroy_delayed_work_on_stack(struct delayed_work *work) { } static
> inline unsigned int work_static(struct work_struct *work) { return 0;
> } #endif
>
> +/**
> + * work_pending - Find out whether a work item is currently pending
> + * @work: The work item in question
> + */
> +#define work_pending(work) \
> + test_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))
> +
> +/**
> + * delayed_work_pending - Find out whether a delayable work item is
> +currently
> + * pending
> + * @w: The work item in question
> + */
> +#define delayed_work_pending(w) \
> + work_pending(&(w)->work)
> +
> +
> /*
> * initialize all of a work item in one go
> *
> @@ -310,6 +326,7 @@ static inline unsigned int work_static(struct
> work_struct *work) { return 0; }
>
> #define __INIT_DELAYED_WORK(_work, _func, _tflags) \
> do { \
> + WARN_ON(delayed_work_pending(_work)); \
How does this work when the data _work points to is not yet initialized?
Reading uninitialized data is UB and may spuriously trigger the warning.
--> The data member in work_struct is a variable of type atomic_long_t, and 0 is the initial value of the atomic variable, right?
--> On the other hand, if a variable of type work_struct is dynamically allocated, an unnecessary warning may be generated randomly; however, it should be a good habit to initialize dynamically allocated variables to 0.
> INIT_WORK(&(_work)->work, (_func)); \
> __init_timer(&(_work)->timer, \
> delayed_work_timer_fn, \
> @@ -318,6 +335,7 @@ static inline unsigned int work_static(struct
> work_struct *work) { return 0; }
[..]
> --
> 2.17.1
>
> ________________________________
> OPPO
The gunk at the end of the mail will prevent your patch from being considered at all.
>
> ±¾µç×ÓÓʼþ¼°Æ丽¼þº¬ÓÐOPPO¹«Ë¾µÄ±£ÃÜÐÅÏ¢£¬½öÏÞÓÚÓʼþÖ¸Ã÷µÄÊÕ¼þÈË£¨°üº¬
> ¸öÈ˼°Èº×飩ʹÓ᣽ûÖ¹ÈκÎÈËÔÚδ¾ÊÚȨµÄÇé¿öÏÂÒÔÈκÎÐÎʽʹÓá£Èç¹ûÄú´í
> ÊÕÁ˱¾Óʼþ£¬ÇÐÎð´«²¥¡¢·Ö·¢¡¢¸´ÖÆ¡¢Ó¡Ë¢»òʹÓñ¾ÓʼþÖ®Èκβ¿·Ö»òÆäËùÔØÖ®
> ÈκÎÄÚÈÝ£¬²¢ÇëÁ¢¼´ÒÔµç×ÓÓʼþ֪ͨ·¢¼þÈ˲¢É¾³ý±¾Óʼþ¼°Æ丽¼þ¡£
[..]
________________________________
OPPO
本电子邮件及其附件含有OPPO公司的保密信息，仅限于邮件指明的收件人（包含个人及群组）使用。禁止任何人在未经授权的情况下以任何形式使用。如果您错收了本邮件，切勿传播、分发、复制、印刷或使用本邮件之任何部分或其所载之任何内容，并请立即以电子邮件通知发件人并删除本邮件及其附件。
网络通讯固有缺陷可能导致邮件被截留、修改、丢失、破坏或包含计算机病毒等不安全情况，OPPO对此类错误或遗漏而引致之任何损失概不承担责任并保留与本邮件相关之一切权利。
除非明确说明，本邮件及其附件无意作为在任何国家或地区之要约、招揽或承诺，亦无意作为任何交易或合同之正式确认。 发件人、其所属机构或所属机构之关联机构或任何上述机构之股东、董事、高级管理人员、员工或其他任何人（以下称“发件人”或“OPPO”）不因本邮件之误送而放弃其所享之任何权利，亦不对因故意或过失使用该等信息而引发或可能引发的损失承担任何责任。
文化差异披露：因全球文化差异影响，单纯以YES\OK或其他简单词汇的回复并不构成发件人对任何交易或合同之正式确认或接受，请与发件人再次确认以获得明确书面意见。发件人不对任何受文化差异影响而导致故意或错误使用该等信息所造成的任何直接或间接损害承担责任。
This e-mail and its attachments contain confidential information from OPPO, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you are not the intended recipient, please do not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.
Electronic communications may contain computer viruses or other defects inherently, may not be accurately and/or timely transmitted to other systems, or may be intercepted, modified ,delayed, deleted or interfered. OPPO shall not be liable for any damages that arise or may arise from such matter and reserves all rights in connection with the email.
Unless expressly stated, this e-mail and its attachments are provided without any warranty, acceptance or promise of any kind in any country or region, nor constitute a formal confirmation or acceptance of any transaction or contract. The sender, together with its affiliates or any shareholder, director, officer, employee or any other person of any such institution (hereinafter referred to as "sender" or "OPPO") does not waive any rights and shall not be liable for any damages that arise or may arise from the intentional or negligent use of such information.
Cultural Differences Disclosure: Due to global cultural differences, any reply with only YES\OK or other simple words does not constitute any confirmation or acceptance of any transaction or contract, please confirm with the sender again to ensure clear opinion in written form. The sender shall not be responsible for any direct or indirect damages resulting from the intentional or misuse of such information.
Hello,
On Wed, Nov 15, 2023 at 07:34:27PM +0800, huangzaiyang wrote:
> @@ -310,6 +326,7 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
>
> #define __INIT_DELAYED_WORK(_work, _func, _tflags) \
> do { \
> + WARN_ON(delayed_work_pending(_work)); \
> INIT_WORK(&(_work)->work, (_func)); \
> __init_timer(&(_work)->timer, \
> delayed_work_timer_fn, \
As pointed out by others, as we don't know what's in the memory, this
wouldn't really work. This kind of bugs should be caught by
DEBUG_OBJECTS_WORK / DEBUG_OBJECTS_TIMERS, I think. It's not perfect as
those aren't usually turned on but catching this sort of errors does need
tracking some information out of band.
Thanks.
--
tejun
On 2023-11-16 01:53:30+0000, 黄再漾(Joyyoung Huang) wrote:
>
> On 2023-11-15 19:34:27+0800, huangzaiyang wrote:
> > There is a timer_list race condition if a delay work is queued twice,
> > this usually won't happen unless someone reinitializes the task before performing the enqueue operation,likeï¼?
> > https://github.com/torvalds/linux/blob/master/drivers/devfreq/devfreq.
> > c#L487 A warning message will help developers identify this irregular
> > usage.
> >
> > Signed-off-by: ZaiYang Huang <[email protected]>
> > ---
> > include/linux/workqueue.h | 33 ++++++++++++++++++---------------
> > 1 file changed, 18 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> > index 24b1e5070f4d..54102ed794e5 100644
> > --- a/include/linux/workqueue.h
> > +++ b/include/linux/workqueue.h
> > @@ -266,6 +266,22 @@ static inline void
> > destroy_delayed_work_on_stack(struct delayed_work *work) { } static
> > inline unsigned int work_static(struct work_struct *work) { return 0;
> > } #endif
> >
> > +/**
> > + * work_pending - Find out whether a work item is currently pending
> > + * @work: The work item in question
> > + */
> > +#define work_pending(work) \
> > + test_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))
> > +
> > +/**
> > + * delayed_work_pending - Find out whether a delayable work item is
> > +currently
> > + * pending
> > + * @w: The work item in question
> > + */
> > +#define delayed_work_pending(w) \
> > + work_pending(&(w)->work)
> > +
> > +
> > /*
> > * initialize all of a work item in one go
> > *
> > @@ -310,6 +326,7 @@ static inline unsigned int work_static(struct
> > work_struct *work) { return 0; }
> >
> > #define __INIT_DELAYED_WORK(_work, _func, _tflags) \
> > do { \
> > + WARN_ON(delayed_work_pending(_work)); \
>
> How does this work when the data _work points to is not yet initialized?
> Reading uninitialized data is UB and may spuriously trigger the warning.
>
> --> The data member in work_struct is a variable of type atomic_long_t, and 0 is the initial value of the atomic variable, right?
Please use proper email quoting style, using "> ".
Normal integers are not initialized by default when allocated on the
stack. I don't think atomic_long_t behaves differently.
> --> On the other hand, if a variable of type work_struct is dynamically allocated, an unnecessary warning may be generated randomly; however, it should be a good habit to initialize dynamically allocated variables to 0.
It seems surprising having to initialize a variable to 0 only to
directly afterwards initialize it again with INIT_DELAYED_WORK().
Also even if there was consensus that it should be done this way,
existing users of this macro don't do it, so these would need to be
adapted first.
> > INIT_WORK(&(_work)->work, (_func)); \
> > __init_timer(&(_work)->timer, \
> > delayed_work_timer_fn, \
> > @@ -318,6 +335,7 @@ static inline unsigned int work_static(struct
> > work_struct *work) { return 0; }
>
> [..]
>
> > --
> > 2.17.1
> >
> > ________________________________
> > OPPO
>
> The gunk at the end of the mail will prevent your patch from being considered at all.
>
> >
> > ±¾µç×ÓÓʼþ¼°Æ丽¼þº¬ÓÐOPPO¹«Ë¾µÄ±£ÃÜÐÅÏ¢£¬½öÏÞÓÚÓʼþÖ¸Ã÷µÄÊÕ¼þÈË£¨°üº¬
> > ¸öÈ˼°Èº×飩ʹÓ᣽ûÖ¹ÈκÎÈËÔÚδ¾ÊÚȨµÄÇé¿öÏÂÒÔÈκÎÐÎʽʹÓá£Èç¹ûÄú´í
> > ÊÕÁ˱¾Óʼþ£¬ÇÐÎð´«²¥¡¢·Ö·¢¡¢¸´ÖÆ¡¢Ó¡Ë¢»òʹÓñ¾ÓʼþÖ®Èκβ¿·Ö»òÆäËùÔØÖ®
> > ÈκÎÄÚÈÝ£¬²¢ÇëÁ¢¼´ÒÔµç×ÓÓʼþ֪ͨ·¢¼þÈ˲¢É¾³ý±¾Óʼþ¼°Æ丽¼þ¡£
This stuff really needs to go.
>Hello,
>
>On Wed, Nov 15, 2023 at 07:34:27PM +0800, huangzaiyang wrote:
>> @@ -310,6 +326,7 @@ static inline unsigned int work_static(struct
>> work_struct *work) { return 0; }
>>
>> #define __INIT_DELAYED_WORK(_work, _func, _tflags) \
>> do { \
>> + WARN_ON(delayed_work_pending(_work)); \
>> INIT_WORK(&(_work)->work, (_func)); \
>> __init_timer(&(_work)->timer, \
>> delayed_work_timer_fn, \
>
>As pointed out by others, as we don't know what's in the memory, this wouldn't really work. This kind of bugs should be caught by DEBUG_OBJECTS_WORK / DEBUG_OBJECTS_TIMERS, I think. It's not perfect as those aren't >usually turned on but catching this sort of errors does need tracking some information out of band.
Thanks, The initial values of variables on the heap and stack are indeed random. Give up this modification and will try to use DEBUG_OBJECTS_WORK / DEBUG_OBJECTS_TIMERS to reproduce the problem.
--
huangzaiyang
________________________________
OPPO
???????ʼ????丽??????OPPO??˾?ı?????Ϣ?????????ʼ?ָ?????ռ??ˣ????????˼?Ⱥ?飩ʹ?á???ֹ?κ?????δ????Ȩ???????????κ???ʽʹ?á????????????˱??ʼ??????????????ַ??????ơ?ӡˢ??ʹ?ñ??ʼ?֮?κβ??ֻ???????֮?κ????ݣ??????????Ե????ʼ?֪ͨ?????˲?ɾ?????ʼ????丽????
????ͨѶ????ȱ?ݿ??ܵ????ʼ??????????ġ???ʧ???ƻ??????????????????Ȳ???ȫ??????OPPO?Դ???????????©??????֮?κ???ʧ?Ų??е????β??????뱾?ʼ?????֮һ??Ȩ????
??????ȷ˵???????ʼ????丽????????Ϊ???κι??һ?????֮ҪԼ??????????ŵ??????????Ϊ?κν?????֮ͬ??ʽȷ?ϡ? ?????ˡ?????????????????????֮???????????κ?????????֮?ɶ??????¡?????????Ա??Ա?????????κ??ˣ????³ơ??????ˡ?????OPPO???????????ʼ?֮???Ͷ???????????֮?κ?Ȩ?????????????????ʧʹ?øõ???Ϣ????????????????????ʧ?е??κ????Ρ?
?Ļ???????¶????ȫ???Ļ?????Ӱ?죬??????YES\OK???????????ʻ??Ļظ????????ɷ????˶??κν?????֮ͬ??ʽȷ?ϻ????ܣ????뷢?????ٴ?ȷ???Ի?????ȷ???????????????˲????κ????Ļ?????Ӱ???????¹?????????ʹ?øõ???Ϣ?????ɵ??κ?ֱ?ӻ??????????е????Ρ?
This e-mail and its attachments contain confidential information from OPPO, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you are not the intended recipient, please do not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.
Electronic communications may contain computer viruses or other defects inherently, may not be accurately and/or timely transmitted to other systems, or may be intercepted, modified ,delayed, deleted or interfered. OPPO shall not be liable for any damages that arise or may arise from such matter and reserves all rights in connection with the email.
Unless expressly stated, this e-mail and its attachments are provided without any warranty, acceptance or promise of any kind in any country or region, nor constitute a formal confirmation or acceptance of any transaction or contract. The sender, together with its affiliates or any shareholder, director, officer, employee or any other person of any such institution (hereinafter referred to as "sender" or "OPPO") does not waive any rights and shall not be liable for any damages that arise or may arise from the intentional or negligent use of such information.
Cultural Differences Disclosure: Due to global cultural differences, any reply with only YES\OK or other simple words does not constitute any confirmation or acceptance of any transaction or contract, please confirm with the sender again to ensure clear opinion in written form. The sender shall not be responsible for any direct or indirect damages resulting from the intentional or misuse of such information.