On 4/18/18 3:18 AM, jiang.biao2@xxxxxxxxxx wrote: > Hi, >>> Il giorno 17 apr 2018, alle ore 09:10, Jiang Biao <jiang.biao2@xxxxxxxxxx> ha scritto: >>> >>> As described in the comment of blkcg_activate_policy(), >>> *Update of each blkg is protected by both queue and blkcg locks so >>> that holding either lock and testing blkcg_policy_enabled() is >>> always enough for dereferencing policy data.* >>> with queue lock held, there is no need to hold blkcg lock in >>> blkcg_deactivate_policy(). Similar case is in >>> blkcg_activate_policy(), which has removed holding of blkcg lock in >>> commit 4c55f4f9ad3001ac1fefdd8d8ca7641d18558e23. >>> >> >> Hi, >> by chance, did you check whether this may cause problems with bfq, >> being the latter not protected by the queue lock as cfq? > Checked the bfq code, bfq seems never used blkcg lock derectly, and > update of blkg in the common code is protected by both queue and > blkcg locks, so IMHO this patch would not introduce any new problem > with bfq, even though bfq is not protected by queue lock. > On the other hand, the locks (queue lock/blkcg lock) used to protected > the update of blkg seems a bit too heavyweight, especially the queue lock > which is used too widely may cause races with other contexts. I wonder > if there is any way to ease the case? e.g. add a new lock for blkg's own.:) It might make sense to lock it separately, but I would not worry about it unless it shows up as hot in your testing. I've applied your patch, thanks. -- Jens Axboe
