Re: [Outreachy kernel] [PATCH v2] staging: comedi: comedi_pcmcia: Removes unnecessary blank line
On Tue, 13 Sep 2016, Namrata A Shettar wrote: > This patch removes an unnecessary blank line that caused checkpatch issue. Actually, commit messages and subject lines should be written in the imperative. So you should say eg Remove unnecessary blank line instead of Removes unnecessary blank line julia > > Signed-off-by: Namrata A Shettar > --- > Changes in v2: > - Changed the subject line > - Changed description of patch > > drivers/staging/comedi/comedi_pcmcia.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/staging/comedi/comedi_pcmcia.c > b/drivers/staging/comedi/comedi_pcmcia.c > index d7072a5..ec8a0ad 100644 > --- a/drivers/staging/comedi/comedi_pcmcia.c > +++ b/drivers/staging/comedi/comedi_pcmcia.c > @@ -18,7 +18,6 @@ > > #include > #include > - > #include "comedi_pcmcia.h" > > /** > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web > visithttps://groups.google.com/d/msgid/outreachy-kernel/CAFrQyDErWX1NgsruZtt10hi > hZR2J3cvK-Q9Zj_dp_GLc_0mLmA%40mail.gmail.com. > For more options, visit https://groups.google.com/d/optout. > >___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: Disable preemption while holding the global binder lock
On Mon, Sep 12, 2016 at 08:44:09PM -0700, Arve Hjønnevåg wrote: > A previous attempt to fix this problem, changed the lock to use > rt_mutex instead of mutex, but this apparently did not work as well as > this patch. I believe the added overhead was noticeable, and it did > not work when the preempted thread was in a different cgroup (I don't > know if this is still the case). Do you actually have RR/FIFO/DL tasks? Currently PI isn't defined/implemented for OTHER. cgroups should be irrelevant, PI is unaware of them. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [PATCH v2] staging: comedi: comedi_pcmcia: Removes unnecessary blank line
On Tuesday, September 13, 2016 11:51:53 AM CEST Namrata A Shettar wrote: > --- a/drivers/staging/comedi/comedi_pcmcia.c > +++ b/drivers/staging/comedi/comedi_pcmcia.c > @@ -18,7 +18,6 @@ > > #include > #include > - > #include "comedi_pcmcia.h" > > /** > I would argue that checkpatch is wrong here, it's very common to have an empty line between the global and the local header files. Arnd ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE
Hi Laura, On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote: > > arm64 may need to guarantee the caches are synced. Implement versions of > the kernel_force_cache API to allow this. > > Signed-off-by: Laura Abbott > --- > v3: Switch to calling cache operations directly instead of relying on > DMA mapping. > --- > arch/arm64/include/asm/cacheflush.h | 8 > arch/arm64/mm/cache.S | 24 > arch/arm64/mm/flush.c | 11 +++ > 3 files changed, 39 insertions(+), 4 deletions(-) I'm really hesitant to expose these cache routines as an API solely to support a driver sitting in staging/. I appreciate that there's a chicken and egg problem here, but we *really* don't want people using these routines in preference to the DMA API, and I fear that we'll simply grow a bunch more users of these things if we promote it as an API like you're proposing. Can the code not be contained under staging/, as part of ion? Will ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [PATCH v2] staging: comedi: comedi_pcmcia: Removes unnecessary blank line
On Tue, 13 Sep 2016, Arnd Bergmann wrote: > On Tuesday, September 13, 2016 11:51:53 AM CEST Namrata A Shettar wrote: > > --- a/drivers/staging/comedi/comedi_pcmcia.c > > +++ b/drivers/staging/comedi/comedi_pcmcia.c > > @@ -18,7 +18,6 @@ > > > > #include > > #include > > - > > #include "comedi_pcmcia.h" > > > > /** > > > > I would argue that checkpatch is wrong here, it's very common to have > an empty line between the global and the local header files. I forwarded this to Joe Perches, and he pointed out that checkpatch doesn't give a warning for this. Namrata, what version of the kernel are you using? julia > > Arnd > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/2284008.DIPsHg5UWl%40wuerfel. > For more options, visit https://groups.google.com/d/optout. > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 2/2] pci-hyperv: properly handle device eject
> From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On Behalf
> Of Long Li
> Sent: Tuesday, September 13, 2016 7:54
> ...
> A PCI_EJECT message can arrive at the same time we are calling
> pci_scan_child_bus in the workqueue for the previous PCI_BUS_RELATIONS
> message, in this case we could potentailly modify the bus from two places.
> Properly lock the bus access.
>
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1587,7 +1587,7 @@ static void hv_eject_device_work(struct work_struct
> *work)
> pdev = pci_get_domain_bus_and_slot(hpdev->hbus->sysdata.domain, 0,
>wslot);
> if (pdev) {
> - pci_stop_and_remove_bus_device(pdev);
> + pci_stop_and_remove_bus_device_locked(pdev);
> pci_dev_put(pdev);
> }
The _locked version tries to get the mutex pci_rescan_remove_lock.
But it looks pci_scan_child_bus() doesn't try to get the mutex(?), so how can
this patch make sure the 2 code paths are not running simultaneously?
Thanks,
-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH v1 01/28] kvm: svm: Add support for additional SVM NPF error codes
On Mon, Aug 22, 2016 at 07:23:44PM -0400, Brijesh Singh wrote: > From: Tom Lendacky > > AMD hardware adds two additional bits to aid in nested page fault handling. > > Bit 32 - NPF occurred while translating the guest's final physical address > Bit 33 - NPF occurred while translating the guest page tables > > The guest page tables fault indicator can be used as an aid for nested > virtualization. Using V0 for the host, V1 for the first level guest and > V2 for the second level guest, when both V1 and V2 are using nested paging > there are currently a number of unnecessary instruction emulations. When > V2 is launched shadow paging is used in V1 for the nested tables of V2. As > a result, KVM marks these pages as RO in the host nested page tables. When > V2 exits and we resume V1, these pages are still marked RO. > > Every nested walk for a guest page table is treated as a user-level write > access and this causes a lot of NPFs because the V1 page tables are marked > RO in the V0 nested tables. While executing V1, when these NPFs occur KVM > sees a write to a read-only page, emulates the V1 instruction and unprotects > the page (marking it RW). This patch looks for cases where we get a NPF due > to a guest page table walk where the page was marked RO. It immediately > unprotects the page and resumes the guest, leading to far fewer instruction > emulations when nested virtualization is used. > > Signed-off-by: Tom Lendacky > --- > arch/x86/include/asm/kvm_host.h | 11 ++- > arch/x86/kvm/mmu.c | 20 ++-- > arch/x86/kvm/svm.c |2 +- > 3 files changed, 29 insertions(+), 4 deletions(-) FWIW: Reviewed-by: Borislav Petkov -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [PATCH v2] staging: comedi: comedi_pcmcia: Removes unnecessary blank line
On Tue, 13 Sep 2016, Namrata A Shettar wrote: > Yes I realize that this may be wrong.Thank you for your inputs! > > Also,The version of the kernel I am using is : 4.8.0-rc2+ You should be using what you get by doing the following command: git clone -b staging-testing git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git Maybe this is what you already have? julia > > Thanks, > Namrata > > On Tuesday, September 13, 2016 at 3:06:06 PM UTC+5:30, Julia Lawall wrote: > > > On Tue, 13 Sep 2016, Arnd Bergmann wrote: > > > On Tuesday, September 13, 2016 11:51:53 AM CEST Namrata A > Shettar wrote: > > > --- a/drivers/staging/comedi/comedi_pcmcia.c > > > +++ b/drivers/staging/comedi/comedi_pcmcia.c > > > @@ -18,7 +18,6 @@ > > > > > > #include > > > #include > > > - > > > #include "comedi_pcmcia.h" > > > > > > /** > > > > > > > I would argue that checkpatch is wrong here, it's very common > to have > > an empty line between the global and the local header files. > > I forwarded this to Joe Perches, and he pointed out that > checkpatch > doesn't give a warning for this. Namrata, what version of the > kernel are > you using? > > julia > > > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web > visithttps://groups.google.com/d/msgid/outreachy-kernel/e2fdc076-900a-4a25-9e5a- > 49d693bd3c74%40googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > >___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
非结构化面试与结构化面试
250 2016-9-13 17:54:12 非结构化面试与结构化面试.xls Description: Binary data ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi_fops: fix checkpatch permission warnings
Fix the following checkpatch warning:
Symbolic permissions are not preferred, use octal permissions.
Signed-off-by: Simon Chopin
---
drivers/staging/comedi/comedi_fops.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 1999eed..bf922ea 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -81,20 +81,20 @@ struct comedi_file {
(COMEDI_NUM_MINORS - COMEDI_NUM_BOARD_MINORS)
static int comedi_num_legacy_minors;
-module_param(comedi_num_legacy_minors, int, S_IRUGO);
+module_param(comedi_num_legacy_minors, int, 0444);
MODULE_PARM_DESC(comedi_num_legacy_minors,
"number of comedi minor devices to reserve for
non-auto-configured devices (default 0)"
);
unsigned int comedi_default_buf_size_kb = CONFIG_COMEDI_DEFAULT_BUF_SIZE_KB;
-module_param(comedi_default_buf_size_kb, uint, S_IRUGO | S_IWUSR);
+module_param(comedi_default_buf_size_kb, uint, 0644);
MODULE_PARM_DESC(comedi_default_buf_size_kb,
"default asynchronous buffer size in KiB (default "
__MODULE_STRING(CONFIG_COMEDI_DEFAULT_BUF_SIZE_KB) ")");
unsigned int comedi_default_buf_maxsize_kb
= CONFIG_COMEDI_DEFAULT_BUF_MAXSIZE_KB;
-module_param(comedi_default_buf_maxsize_kb, uint, S_IRUGO | S_IWUSR);
+module_param(comedi_default_buf_maxsize_kb, uint, 0644);
MODULE_PARM_DESC(comedi_default_buf_maxsize_kb,
"default maximum size of asynchronous buffer in KiB (default "
__MODULE_STRING(CONFIG_COMEDI_DEFAULT_BUF_MAXSIZE_KB) ")");
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi_fops: fix checkpatch permission warnings
On Sat, Sep 10, 2016 at 05:23:57AM +0200, Simon Chopin wrote: > Fix the following checkpatch warning: > Symbolic permissions are not preferred, use octal permissions. > > Signed-off-by: Simon Chopin > --- > drivers/staging/comedi/comedi_fops.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Someone else sent this same patch right before you did, sorry :( greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi_fops: fix checkpatch permission warnings
On Tue, Sep 13, 2016 at 2:52 PM, Greg Kroah-Hartman wrote: > On Sat, Sep 10, 2016 at 05:23:57AM +0200, Simon Chopin wrote: >> Fix the following checkpatch warning: >> Symbolic permissions are not preferred, use octal permissions. >> >> Signed-off-by: Simon Chopin >> --- >> drivers/staging/comedi/comedi_fops.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) > > Someone else sent this same patch right before you did, sorry :( Eh, it was bound to happen, especially a whole bunch of the eudyptula challengers have/will reach the "submit a cleanup patch" task in the coming days/weeks. At least I got the submission part right, didn't I? Cheers, Simon ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi_fops: fix checkpatch permission warnings
On 13/09/16 13:59, Simon Chopin wrote: On Tue, Sep 13, 2016 at 2:52 PM, Greg Kroah-Hartman wrote: On Sat, Sep 10, 2016 at 05:23:57AM +0200, Simon Chopin wrote: Fix the following checkpatch warning: Symbolic permissions are not preferred, use octal permissions. Signed-off-by: Simon Chopin --- drivers/staging/comedi/comedi_fops.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Someone else sent this same patch right before you did, sorry :( Eh, it was bound to happen, especially a whole bunch of the eudyptula challengers have/will reach the "submit a cleanup patch" task in the coming days/weeks. At least I got the submission part right, didn't I? Yes, the submission was fine! -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi_fops: fix checkpatch permission warnings
On Tue, Sep 13, 2016 at 02:59:36PM +0200, Simon Chopin wrote: > On Tue, Sep 13, 2016 at 2:52 PM, Greg Kroah-Hartman > wrote: > > On Sat, Sep 10, 2016 at 05:23:57AM +0200, Simon Chopin wrote: > >> Fix the following checkpatch warning: > >> Symbolic permissions are not preferred, use octal permissions. > >> > >> Signed-off-by: Simon Chopin > >> --- > >> drivers/staging/comedi/comedi_fops.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > > > > Someone else sent this same patch right before you did, sorry :( > > Eh, it was bound to happen, especially a whole bunch of the eudyptula > challengers have/will reach > the "submit a cleanup patch" task in the coming days/weeks. > > At least I got the submission part right, didn't I? Yes, everything looked correct. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fwserial: fix checkpatch permission warnings
Fix the following warnings: Symbolic permissions are not preferred. Consider using octal permissions. Signed-off-by: Simon Chopin --- drivers/staging/fwserial/fwserial.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c index c241c0a..49c718b 100644 --- a/drivers/staging/fwserial/fwserial.c +++ b/drivers/staging/fwserial/fwserial.c @@ -39,9 +39,9 @@ static int num_ttys = 4; /* # of std ttys to create per fw_card*/ static bool auto_connect = true;/* try to VIRT_CABLE to every peer */ static bool create_loop_dev = true; /* create a loopback device for each card */ -module_param_named(ttys, num_ttys, int, S_IRUGO | S_IWUSR); -module_param_named(auto, auto_connect, bool, S_IRUGO | S_IWUSR); -module_param_named(loop, create_loop_dev, bool, S_IRUGO | S_IWUSR); +module_param_named(ttys, num_ttys, int, 0644); +module_param_named(auto, auto_connect, bool, 0644); +module_param_named(loop, create_loop_dev, bool, 0644); /* * Threshold below which the tty is woken for writing -- 2.1.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE
On 09/13/2016 02:19 AM, Will Deacon wrote: Hi Laura, On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote: arm64 may need to guarantee the caches are synced. Implement versions of the kernel_force_cache API to allow this. Signed-off-by: Laura Abbott --- v3: Switch to calling cache operations directly instead of relying on DMA mapping. --- arch/arm64/include/asm/cacheflush.h | 8 arch/arm64/mm/cache.S | 24 arch/arm64/mm/flush.c | 11 +++ 3 files changed, 39 insertions(+), 4 deletions(-) I'm really hesitant to expose these cache routines as an API solely to support a driver sitting in staging/. I appreciate that there's a chicken and egg problem here, but we *really* don't want people using these routines in preference to the DMA API, and I fear that we'll simply grow a bunch more users of these things if we promote it as an API like you're proposing. Can the code not be contained under staging/, as part of ion? I proposed that in V1 and it was suggested I make it a proper API http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47654.html http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47672.html Will Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE
On Tue, Sep 13, 2016 at 08:02:20AM -0700, Laura Abbott wrote: > On 09/13/2016 02:19 AM, Will Deacon wrote: > >On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote: > >> > >>arm64 may need to guarantee the caches are synced. Implement versions of > >>the kernel_force_cache API to allow this. > >> > >>Signed-off-by: Laura Abbott > >>--- > >>v3: Switch to calling cache operations directly instead of relying on > >>DMA mapping. > >>--- > >> arch/arm64/include/asm/cacheflush.h | 8 > >> arch/arm64/mm/cache.S | 24 > >> arch/arm64/mm/flush.c | 11 +++ > >> 3 files changed, 39 insertions(+), 4 deletions(-) > > > >I'm really hesitant to expose these cache routines as an API solely to > >support a driver sitting in staging/. I appreciate that there's a chicken > >and egg problem here, but we *really* don't want people using these routines > >in preference to the DMA API, and I fear that we'll simply grow a bunch > >more users of these things if we promote it as an API like you're proposing. > > > >Can the code not be contained under staging/, as part of ion? > > > > I proposed that in V1 and it was suggested I make it a proper API > > http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47654.html > http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47672.html :/ then I guess we're in disagreement. If ion really needs this stuff (which I don't fully grok), perhaps we should be exposing something at a higher level from the architecture, so it really can't be used for anything other than ion. Will ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 2/2] pci-hyperv: properly handle device eject
> -Original Message-
> From: Dexuan Cui
> Sent: Tuesday, September 13, 2016 2:51 AM
> To: Long Li ; KY Srinivasan ;
> Haiyang Zhang ; Bjorn Helgaas
>
> Cc: de...@linuxdriverproject.org; linux-ker...@vger.kernel.org; linux-
> p...@vger.kernel.org
> Subject: RE: [PATCH 2/2] pci-hyperv: properly handle device eject
>
> > From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> > Behalf Of Long Li
> > Sent: Tuesday, September 13, 2016 7:54 ...
> > A PCI_EJECT message can arrive at the same time we are calling
> > pci_scan_child_bus in the workqueue for the previous
> PCI_BUS_RELATIONS
> > message, in this case we could potentailly modify the bus from two places.
> > Properly lock the bus access.
> >
> > --- a/drivers/pci/host/pci-hyperv.c
> > +++ b/drivers/pci/host/pci-hyperv.c
> > @@ -1587,7 +1587,7 @@ static void hv_eject_device_work(struct
> > work_struct
> > *work)
> > pdev = pci_get_domain_bus_and_slot(hpdev->hbus->sysdata.domain,
> 0,
> >wslot);
> > if (pdev) {
> > - pci_stop_and_remove_bus_device(pdev);
> > + pci_stop_and_remove_bus_device_locked(pdev);
> > pci_dev_put(pdev);
> > }
>
> The _locked version tries to get the mutex pci_rescan_remove_lock.
>
> But it looks pci_scan_child_bus() doesn't try to get the mutex(?), so how can
> this patch make sure the 2 code paths are not running simultaneously?
Thanks for the review.
The lock is to protect the following call to pci_scan_child_bus() in
pci_devices_present_work():
/*
* Tell the core to rescan bus
* because there may have been changes.
*/
pci_lock_rescan_remove();
pci_scan_child_bus(hbus->pci_bus);
pci_unlock_rescan_remove();
This race condition has shown up in the tests.
You raised a valid concern in create_root_hv_pci_bus(). There might be another
race condition there. I'll look into this.
>
> Thanks,
> -- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 2/2] pci-hyperv: properly handle device eject
> -Original Message-
> From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> Behalf Of Long Li
> Sent: Tuesday, September 13, 2016 10:33 AM
> To: Dexuan Cui ; KY Srinivasan
> ; Haiyang Zhang ; Bjorn
> Helgaas
> Cc: de...@linuxdriverproject.org; linux-ker...@vger.kernel.org; linux-
> p...@vger.kernel.org
> Subject: RE: [PATCH 2/2] pci-hyperv: properly handle device eject
>
> This sender failed our fraud detection checks and may not be who they
> appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing
>
> > -Original Message-
> > From: Dexuan Cui
> > Sent: Tuesday, September 13, 2016 2:51 AM
> > To: Long Li ; KY Srinivasan ;
> > Haiyang Zhang ; Bjorn Helgaas
> >
> > Cc: de...@linuxdriverproject.org; linux-ker...@vger.kernel.org; linux-
> > p...@vger.kernel.org
> > Subject: RE: [PATCH 2/2] pci-hyperv: properly handle device eject
> >
> > > From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org]
> > > On Behalf Of Long Li
> > > Sent: Tuesday, September 13, 2016 7:54 ...
> > > A PCI_EJECT message can arrive at the same time we are calling
> > > pci_scan_child_bus in the workqueue for the previous
> > PCI_BUS_RELATIONS
> > > message, in this case we could potentailly modify the bus from two
> places.
> > > Properly lock the bus access.
> > >
> > > --- a/drivers/pci/host/pci-hyperv.c
> > > +++ b/drivers/pci/host/pci-hyperv.c
> > > @@ -1587,7 +1587,7 @@ static void hv_eject_device_work(struct
> > > work_struct
> > > *work)
> > > pdev =
> > > pci_get_domain_bus_and_slot(hpdev->hbus->sysdata.domain,
> > 0,
> > >wslot);
> > > if (pdev) {
> > > - pci_stop_and_remove_bus_device(pdev);
> > > + pci_stop_and_remove_bus_device_locked(pdev);
> > > pci_dev_put(pdev);
> > > }
> >
> > The _locked version tries to get the mutex pci_rescan_remove_lock.
> >
> > But it looks pci_scan_child_bus() doesn't try to get the mutex(?), so
> > how can this patch make sure the 2 code paths are not running
> simultaneously?
>
> Thanks for the review.
>
> The lock is to protect the following call to pci_scan_child_bus() in
> pci_devices_present_work():
>
> /*
> * Tell the core to rescan bus
> * because there may have been changes.
> */
> pci_lock_rescan_remove();
> pci_scan_child_bus(hbus->pci_bus);
> pci_unlock_rescan_remove();
>
> This race condition has shown up in the tests.
>
> You raised a valid concern in create_root_hv_pci_bus(). There might be
> another race condition there. I'll look into this.
I think this code is safe here. If we reach the code
pci_stop_and_remove_bus_device_locked, create_root_hv_pci_bus() is already
called.
>
> >
> > Thanks,
> > -- Dexuan
> ___
> devel mailing list
> de...@linuxdriverproject.org
> https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fdriverde
> v.linuxdriverproject.org%2fmailman%2flistinfo%2fdriverdev-
> devel&data=02%7c01%7clongli%40microsoft.com%7c3d12ee6d87c140eb5114
> 08d3dbfc1713%7c72f988bf86f141af91ab2d7cd011db47%7c1%7c0%7c6360938
> 48185348266&sdata=a2GYqIBsQAFxszkKg3fl1nqqPgvZHh%2bAY2255RgrvUU
> %3d
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] Drivers: hv: utils: Support TimeSync version 4.0 protocol samples.
On Thu, Sep 08, k...@exchange.microsoft.com wrote: > - default: > + case(VERSION_WIN10): > util_fw_version = UTIL_FW_VERSION; > sd_srv_version = SD_VERSION; > ts_srv_version = TS_VERSION; > hb_srv_version = HB_VERSION; > + break; > + default: > + util_fw_version = UTIL_FW_VERSION; > + sd_srv_version = SD_VERSION; > + ts_srv_version = TS_VERSION_3; > + hb_srv_version = HB_VERSION; Is this correct? An old kernel on a newer host would use the old protocol. I assume that new host will also know about the old protocol? Perhaps a better approach would be to list the known existing hosts and use the new protocol for upcoming, unknown hosts via 'default:'. Olaf signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 3/3] Drivers: hv: utils: Support TimeSync version 4.0 protocol samples.
> On Thu, Sep 08, k...@exchange.microsoft.com wrote: > > > - default: > > + case(VERSION_WIN10): > > util_fw_version = UTIL_FW_VERSION; > > sd_srv_version = SD_VERSION; > > ts_srv_version = TS_VERSION; > > hb_srv_version = HB_VERSION; > > + break; > > + default: > > + util_fw_version = UTIL_FW_VERSION; > > + sd_srv_version = SD_VERSION; > > + ts_srv_version = TS_VERSION_3; > > + hb_srv_version = HB_VERSION; > > Is this correct? An old kernel on a newer host would use the old protocol. I > assume that new host will also know about the old protocol? This is correct. An old kernel uses the old protocol even with the new host. New hosts understand the old protocol. > Perhaps a better approach would be to list the known existing hosts and use > the new protocol for upcoming, unknown hosts via 'default:'. This is a good idea. I will create another patch that addresses this. > > Olaf ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE
On 09/13/2016 08:14 AM, Will Deacon wrote: On Tue, Sep 13, 2016 at 08:02:20AM -0700, Laura Abbott wrote: On 09/13/2016 02:19 AM, Will Deacon wrote: On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote: arm64 may need to guarantee the caches are synced. Implement versions of the kernel_force_cache API to allow this. Signed-off-by: Laura Abbott --- v3: Switch to calling cache operations directly instead of relying on DMA mapping. --- arch/arm64/include/asm/cacheflush.h | 8 arch/arm64/mm/cache.S | 24 arch/arm64/mm/flush.c | 11 +++ 3 files changed, 39 insertions(+), 4 deletions(-) I'm really hesitant to expose these cache routines as an API solely to support a driver sitting in staging/. I appreciate that there's a chicken and egg problem here, but we *really* don't want people using these routines in preference to the DMA API, and I fear that we'll simply grow a bunch more users of these things if we promote it as an API like you're proposing. Can the code not be contained under staging/, as part of ion? I proposed that in V1 and it was suggested I make it a proper API http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47654.html http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47672.html :/ then I guess we're in disagreement. If ion really needs this stuff (which I don't fully grok), perhaps we should be exposing something at a higher level from the architecture, so it really can't be used for anything other than ion. I talked/complained about this at a past plumbers. The gist is that Ion ends up acting as a fake DMA layer for clients. It doesn't match nicely because clients can allocate both coherent and non-coherent memory. Trying to use dma_map doesn't work because a) a device for coherency isn't known at allocation time b) it kills performance. Part of the motivation for taking this approach is to avoid the need to rework the existing Android userspace and keep the existing behavior, as terrible as it is. Having Ion out of staging and not actually usable isn't helpful. I'll give this all some more thought and hopefully have one or two more proposals before Connect/Plumbers. Will Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: octeon: use defines instead of magic numbers
The ugly magic number 65392 is waiting for CVMX_IPD_MAX_MTU
to appear in the mips tree.
Signed-off-by: Asbjoern Sloth Toennesen
---
drivers/staging/octeon/ethernet.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/octeon/ethernet.c
b/drivers/staging/octeon/ethernet.c
index 5f746b8..790477c 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -17,6 +17,8 @@
#include
#include
#include
+#include
+#include
#include
@@ -39,6 +41,8 @@
#include
#include
+#define OCTEON_MAX_MTU 65392
+
static int num_packet_buffers = 1024;
module_param(num_packet_buffers, int, 0444);
MODULE_PARM_DESC(num_packet_buffers, "\n"
@@ -249,19 +253,21 @@ static int cvm_oct_common_change_mtu(struct net_device
*dev, int new_mtu)
struct octeon_ethernet *priv = netdev_priv(dev);
int interface = INTERFACE(priv->port);
#if IS_ENABLED(CONFIG_VLAN_8021Q)
- int vlan_bytes = 4;
+ int vlan_bytes = VLAN_HLEN;
#else
int vlan_bytes = 0;
#endif
+ int mtu_overhead = ETH_HLEN + ETH_FCS_LEN + vlan_bytes;
/*
* Limit the MTU to make sure the ethernet packets are between
* 64 bytes and 65535 bytes.
*/
- if ((new_mtu + 14 + 4 + vlan_bytes < 64) ||
- (new_mtu + 14 + 4 + vlan_bytes > 65392)) {
+ if ((new_mtu + mtu_overhead < VLAN_ETH_ZLEN) ||
+ (new_mtu + mtu_overhead > OCTEON_MAX_MTU)) {
pr_err("MTU must be between %d and %d.\n",
- 64 - 14 - 4 - vlan_bytes, 65392 - 14 - 4 - vlan_bytes);
+ VLAN_ETH_ZLEN - mtu_overhead,
+ OCTEON_MAX_MTU - mtu_overhead);
return -EINVAL;
}
dev->mtu = new_mtu;
@@ -271,7 +277,7 @@ static int cvm_oct_common_change_mtu(struct net_device
*dev, int new_mtu)
CVMX_HELPER_INTERFACE_MODE_SPI)) {
int index = INDEX(priv->port);
/* Add ethernet header and FCS, and VLAN if configured. */
- int max_packet = new_mtu + 14 + 4 + vlan_bytes;
+ int max_packet = new_mtu + mtu_overhead;
if (OCTEON_IS_MODEL(OCTEON_CN3XXX) ||
OCTEON_IS_MODEL(OCTEON_CN58XX)) {
@@ -286,7 +292,7 @@ static int cvm_oct_common_change_mtu(struct net_device
*dev, int new_mtu)
union cvmx_pip_frm_len_chkx frm_len_chk;
frm_len_chk.u64 = 0;
- frm_len_chk.s.minlen = 64;
+ frm_len_chk.s.minlen = VLAN_ETH_ZLEN;
frm_len_chk.s.maxlen = max_packet;
cvmx_write_csr(CVMX_PIP_FRM_LEN_CHKX(interface),
frm_len_chk.u64);
--
2.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: Disable preemption while holding the global binder lock
On Mon, Sep 12, 2016 at 11:42 PM, Greg Kroah-Hartman wrote: > On Mon, Sep 12, 2016 at 08:44:09PM -0700, Arve Hjønnevåg wrote: >> On Sat, Sep 10, 2016 at 10:28 AM, Greg Kroah-Hartman >> wrote: >> > On Sat, Sep 10, 2016 at 06:37:29PM +0200, Thomas Gleixner wrote: >> >> On Sat, 10 Sep 2016, Peter Zijlstra wrote: >> >> >> >> > On Sat, Sep 10, 2016 at 09:16:59AM -0700, Christoph Hellwig wrote: >> >> > > On Thu, Sep 08, 2016 at 09:12:50AM -0700, Todd Kjos wrote: >> >> > > > In Android systems, the display pipeline relies on low >> >> > > > latency binder transactions and is therefore sensitive to >> >> > > > delays caused by contention for the global binder lock. >> >> > > > Jank is siginificantly reduced by disabling preemption >> >> > > > while the global binder lock is held. >> >> > > >> >> > > That's now how preempt_disable is supposed to use. It is for critical >> >> > >> >> > not, that's supposed to be _not_. Just to be absolutely clear, this is >> >> > NOT how you're supposed to use preempt_disable(). >> >> > >> >> > > sections that use per-cpu or similar resources. >> >> > > >> >> > > > >> >> > > > Originally-from: Riley Andrews >> >> > > > Signed-off-by: Todd Kjos >> >> > >> >> > > > @@ -389,7 +390,11 @@ static int task_get_unused_fd_flags(struct >> >> > > > binder_proc *proc, int flags) >> >> > > > rlim_cur = task_rlimit(proc->tsk, RLIMIT_NOFILE); >> >> > > > unlock_task_sighand(proc->tsk, &irqs); >> >> > > > >> >> > > > - return __alloc_fd(files, 0, rlim_cur, flags); >> >> > > > + preempt_enable_no_resched(); >> >> > > > + ret = __alloc_fd(files, 0, rlim_cur, flags); >> >> > > > + preempt_disable(); >> >> > >> >> > And the fact that people want to use preempt_enable_no_resched() shows >> >> > that they're absolutely clueless. >> >> > >> >> > This is so broken its not funny. >> >> > >> >> > NAK NAK NAK >> >> >> >> Indeed. Sprinkling random preempt_enabe/disable() pairs all over the place >> >> documents clearly that this is tinkering and not proper software >> >> engineering. >> > >> > I have pointed out in the other thread for this patch (the one that had >> > a patch that could be applied) that the single lock in the binder code >> > is the main problem here, it should be solved instead of this messing >> > around with priorities. >> > >> >> While removing the single lock in the binder driver would help reduce >> the problem that this patch tries to work around, it would not fix it. >> The largest problems occur when a very low priority thread gets >> preempted while holding the lock. When a high priority thread then >> needs the same lock it can't get it. Changing the driver to use more >> fine-grained locking would reduce the set of threads that can trigger >> this problem, but there are processes that receive work from both high >> and low priority threads and could still end up in the same situation. > > Ok, but how would this patch solve the problem? It only reduces the > window of when the preemption could occur, it doesn't stop it from > happening entirely. > I agree, this patch does not _solve_ the problem either. It only reduces it, as there are many places where it re-enables preemption for significant work. >> A previous attempt to fix this problem, changed the lock to use >> rt_mutex instead of mutex, but this apparently did not work as well as >> this patch. I believe the added overhead was noticeable, and it did >> not work when the preempted thread was in a different cgroup (I don't >> know if this is still the case). > > Why would rt_mutex solve this? If the task that holds the lock would run when you try to get the lock, there would be no long delay to get the lock. rt_mutex seems to be designed to solve this problem. > > I worry that this is only going to get worse as more portions of the > Android userspace/HAL get rewritten to use binder more and more. What > about trying to get rid of the lock entirely? That way the task > priority levels would work "properly" here. > I think a good first step would be to switch to locks with a smaller scope. I'm not how useful it would be to eliminate locks in this driver since it calls into other code that uses locks. > thanks, > > greg k-h -- Arve Hjønnevåg ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: Disable preemption while holding the global binder lock
On Tue, Sep 13, 2016 at 12:32 AM, Peter Zijlstra wrote: > On Mon, Sep 12, 2016 at 08:44:09PM -0700, Arve Hjønnevåg wrote: > >> A previous attempt to fix this problem, changed the lock to use >> rt_mutex instead of mutex, but this apparently did not work as well as >> this patch. I believe the added overhead was noticeable, and it did >> not work when the preempted thread was in a different cgroup (I don't >> know if this is still the case). > > Do you actually have RR/FIFO/DL tasks? Currently PI isn't > defined/implemented for OTHER. > Most of the tasks here are not RR/FIFO/DL tasks. I don't see anything in the rtmutex code or documentation that indicates that they don't work for normal tasks. From what I can tell the priority gets boosted in every case. This may not work as well for CFS tasks as for realtime tasks, but it should at least help when there is a large priority difference. > cgroups should be irrelevant, PI is unaware of them. I don't think cgroups are irrelevant. PI being unaware of them explains the problem I described. If the task that holds the lock is in a cgroup that has a low cpu.shares value, then boosting the task's priority within that group does necessarily make it any more likely to run. -- Arve Hjønnevåg ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: lustre/ldlm: Fixed sparse warnings
On Sep 12, 2016, at 04:27, Greg KH wrote: > > On Fri, Sep 09, 2016 at 08:50:35PM +0530, Nayeemahmed Badebade wrote: >> Added __acquires / __releases sparse locking annotations >> to lock_res_and_lock and unlock_res_and_lock functions in >> l_lock.c, to fix below sparse warnings: >> >> l_lock.c:47:22: warning: context imbalance in 'lock_res_and_lock' - wrong >> count at exit >> l_lock.c:62:6: warning: context imbalance in 'unlock_res_and_lock' - >> unexpected unlock >> >> Signed-off-by: Nayeemahmed Badebade >> --- >> drivers/staging/lustre/lustre/ldlm/l_lock.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/staging/lustre/lustre/ldlm/l_lock.c >> b/drivers/staging/lustre/lustre/ldlm/l_lock.c >> index ea8840c..c4b9612 100644 >> --- a/drivers/staging/lustre/lustre/ldlm/l_lock.c >> +++ b/drivers/staging/lustre/lustre/ldlm/l_lock.c >> @@ -45,6 +45,8 @@ >> * being an atomic operation. >> */ >> struct ldlm_resource *lock_res_and_lock(struct ldlm_lock *lock) >> +__acquires(&lock->l_lock) >> +__acquires(lock->l_resource) > > Hm, these are tricky, I don't want to take this type of change without > an ack from the lustre developers... The "__acquires(&lock->l_lock)" line here looks correct, along with the corresponding "__releases(&lock->l_lock)" at unlock_res_and_lock(). The problem, however, is that "l_resource" is not a lock, but rather a struct. The call to "lock_res(lock->l_resource)" is actually locking "lr_lock" internally. It would be better to add "__acquires(&res->lr_lock)" at lock_res() and "__releases(&res->lr_lock)" at unlock_res(). That will also forestall any other warnings about an imbalance with lock_res()/unlock_res() or their callsites. Cheers, Andreas ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 2/2] pci-hyperv: properly handle device eject
> From: Long Li > Sent: Wednesday, September 14, 2016 1:41 > > I think this code is safe here. If we reach the code > pci_stop_and_remove_bus_device_locked, create_root_hv_pci_bus() is already > called. When hv_pci_probe() -> create_root_hv_pci_bus() -> pci_scan_child_bus() is running on one cpu, I think nothing in the current code can prevent hv_eject_device_work() -> pci_stop_and_remove_bus_device_locked() from running on another cpu? The race window is pretty small however. Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
