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
| ||
|
Date: Wed, 2 Dec 2020 09:58:29 -0800 From: "Asutosh Das (asd)" <asutoshd@...eaurora.org> To: Bean Huo <huobean@...il.com>, alim.akhtar@...sung.com, avri.altman@....com, jejb@...ux.ibm.com, martin.petersen@...cle.com, stanley.chu@...iatek.com, beanhuo@...ron.com, bvanassche@....org, tomas.winkler@...el.com, cang@...eaurora.org Cc: linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH 1/3] scsi: ufs: Add "wb_on" sysfs node to control WB on/off On 12/2/2020 8:20 AM, Bean Huo wrote: > On Mon, 2020-11-30 at 15:19 -0800, Asutosh Das (asd) wrote: >>> + return -EINVAL; >>> + >>> + pm_runtime_get_sync(hba->dev); >>> + res = ufshcd_wb_ctrl(hba, wb_enable); >> >> Say, a platform supports clock-scaling and this bit is toggled. >> The control goes into ufshcd_wb_ctrl for both this sysfs and >> clock-scaling contexts. The clock-scaling context passes all checks >> and >> blocks on waiting for this wb control to be disabled and then tries >> to >> enable wb when it's already disabled. Perhaps that's a race there? > > Hi Asutosh > Appreciate your review. > There is only inconsistent problem between clock-scaling and sysfs, > since hba->dev_cmd.lock can garantee there is only one can change > fWriteBoosterEn. But this is only happening on user willfully wants to > control WB through sysfs even they know the platform supports clock- > scaling. > > Since this is for the platform which doesn't support clock-scaling, I > think based on your comments, it should be acceptable for you like > this: Or a synchronization primitive b/w the 2 contexts would work just as well. However, I don't have an issue if the user is denied toggling of wb anyway. LGTM. > > > +static ssize_t wb_on_store(struct device *dev, struct device_attribute > *attr, > + const char *buf, size_t count) > +{ > + struct ufs_hba *hba = dev_get_drvdata(dev); > + unsigned int wb_enable; > + ssize_t res; > + > + if (ufshcd_is_clkscaling_supported(hba)) { > + dev_err(dev, "supports dynamic clk scaling, control WB > + through sysfs is not allowed!"); > + return -EOPNOTSUPP; > + } > + if (!ufshcd_is_wb_allowed(hba)) > + return -EOPNOTSUPP; > + > + if (kstrtouint(buf, 0, &wb_enable)) > + return -EINVAL; > + > + if (wb_enable != 0 && wb_enable != 1) > + return -EINVAL; > + > + pm_runtime_get_sync(hba->dev); > + res = ufshcd_wb_ctrl(hba, wb_enable); > + pm_runtime_put_sync(hba->dev); > + > + return res < 0 ? res : count; > +} > > thanks, > Bean > > -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, Linux Foundation Collaborative Project
Powered by blists - more mailing lists