linux-kernel - Re: [PATCH 30/57] power: ab8500: Flush & sync all works

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] [day] [month] [year] [list]

Message-ID: <20120928195407.GB28243@lizard>
Date:	Fri, 28 Sep 2012 12:54:07 -0700
From:	Anton Vorontsov <anton.vorontsov@...aro.org>
To:	Mathieu Poirier <mathieu.poirier@...aro.org>
Cc:	linux-kernel@...r.kernel.org, dwmw2@...radead.org
Subject: Re: [PATCH 30/57] power: ab8500: Flush & sync all works

On Fri, Sep 28, 2012 at 12:28:03PM -0600, Mathieu Poirier wrote:
[...]
> >> +	flush_delayed_work_sync(&di->attach_work);
> >> +	flush_delayed_work_sync(&di->usb_charger_attached_work);
> >> +	flush_delayed_work_sync(&di->ac_charger_attached_work);
> >> +	flush_delayed_work_sync(&di->check_usbchgnotok_work);
> >> +	flush_delayed_work_sync(&di->check_vbat_work);
> >> +	flush_delayed_work_sync(&di->kick_wd_work);
> >> +
> >> +	flush_work_sync(&di->usb_link_status_work);
> >> +	flush_work_sync(&di->ac_work);
> >> +	flush_work_sync(&di->detect_usb_type_work);
> > 
> > I belive each of these have to be added by the patches that add the
> > appropriate work structs. But really, it's better to avoid these many
> > delayed work.
> 
> Agreed - on the flip side they were added over multiple patches spanning
> many centuries.  Tracking all this work down would be impossible as
> history was re-written many time over.  What would you like to see
> happening here ?

Um.. why can't we just fold

+	flush_delayed_work_sync(&di->attach_work);

Into "[PATCH 20/57] power: Adds support for legacy USB chargers"?

Then 

+	flush_delayed_work_sync(&di->usb_charger_attached_work);
+	flush_delayed_work_sync(&di->ac_charger_attached_work);

Into "[PATCH 07/57] power: ab8500_bm: Detect removed charger"

etc...

That is, at the same time you introduce the work structs, you have to
flush them appropriately.

For work structs that were already in mainline, you'll have this patch to
fixup things. But adding new code with known bugs, and then fixing them in
one go is usually not how things should be done (unless there's a good
technical reason behind this).

Thanks,
Anton.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/