* Re: [PATCH] dax: Change return type to vm_fault_t [not found] <20180414155059.GA18015@jordon-HP-15-Notebook-PC> @ 2018-04-16 16:14 ` Dan Williams 2018-04-16 16:29 ` Randy Dunlap ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Dan Williams @ 2018-04-16 16:14 UTC (permalink / raw) To: Souptick Joarder Cc: linux-nvdimm, Matthew Wilcox, Linux MM, Andrew Morton, Michal Hocko On Sat, Apr 14, 2018 at 8:50 AM, Souptick Joarder <jrdr.linux@gmail.com> wrote: > Use new return type vm_fault_t for fault and > huge_fault handler. > > Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com> > Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com> > --- > drivers/dax/device.c | 26 +++++++++++--------------- > 1 file changed, 11 insertions(+), 15 deletions(-) > > diff --git a/drivers/dax/device.c b/drivers/dax/device.c > index 2137dbc..a122701 100644 > --- a/drivers/dax/device.c > +++ b/drivers/dax/device.c > @@ -243,11 +243,11 @@ __weak phys_addr_t dax_pgoff_to_phys(struct dev_dax *dev_dax, pgoff_t pgoff, > return -1; > } > > -static int __dev_dax_pte_fault(struct dev_dax *dev_dax, struct vm_fault *vmf) > +static vm_fault_t __dev_dax_pte_fault(struct dev_dax *dev_dax, > + struct vm_fault *vmf) > { > struct device *dev = &dev_dax->dev; > struct dax_region *dax_region; > - int rc = VM_FAULT_SIGBUS; > phys_addr_t phys; > pfn_t pfn; > unsigned int fault_size = PAGE_SIZE; > @@ -274,17 +274,11 @@ static int __dev_dax_pte_fault(struct dev_dax *dev_dax, struct vm_fault *vmf) > > pfn = phys_to_pfn_t(phys, dax_region->pfn_flags); > > - rc = vm_insert_mixed(vmf->vma, vmf->address, pfn); > - > - if (rc == -ENOMEM) > - return VM_FAULT_OOM; > - if (rc < 0 && rc != -EBUSY) > - return VM_FAULT_SIGBUS; > - > - return VM_FAULT_NOPAGE; > + return vmf_insert_mixed(vmf->vma, vmf->address, pfn); Ugh, so this change to vmf_insert_mixed() went upstream without fixing the users? This changelog is now misleading as it does not mention that is now an urgent standalone fix. On first read I assumed this was part of a wider effort for 4.18. Grumble, we'll get this applied with a 'Fixes: 1c8f422059ae ("mm: change return type to vm_fault_t")' tag. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dax: Change return type to vm_fault_t 2018-04-16 16:14 ` [PATCH] dax: Change return type to vm_fault_t Dan Williams @ 2018-04-16 16:29 ` Randy Dunlap 2018-04-16 16:38 ` Souptick Joarder 2018-04-16 17:47 ` Matthew Wilcox 2018-04-17 0:14 ` Theodore Y. Ts'o 2 siblings, 1 reply; 13+ messages in thread From: Randy Dunlap @ 2018-04-16 16:29 UTC (permalink / raw) To: Dan Williams, Souptick Joarder Cc: linux-nvdimm, Matthew Wilcox, Linux MM, Andrew Morton, Michal Hocko On 04/16/2018 09:14 AM, Dan Williams wrote: > On Sat, Apr 14, 2018 at 8:50 AM, Souptick Joarder <jrdr.linux@gmail.com> wrote: >> Use new return type vm_fault_t for fault and >> huge_fault handler. >> >> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com> >> Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com> >> --- >> drivers/dax/device.c | 26 +++++++++++--------------- >> 1 file changed, 11 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/dax/device.c b/drivers/dax/device.c >> index 2137dbc..a122701 100644 >> --- a/drivers/dax/device.c >> +++ b/drivers/dax/device.c >> @@ -243,11 +243,11 @@ __weak phys_addr_t dax_pgoff_to_phys(struct dev_dax *dev_dax, pgoff_t pgoff, >> return -1; >> } >> >> -static int __dev_dax_pte_fault(struct dev_dax *dev_dax, struct vm_fault *vmf) >> +static vm_fault_t __dev_dax_pte_fault(struct dev_dax *dev_dax, >> + struct vm_fault *vmf) >> { >> struct device *dev = &dev_dax->dev; >> struct dax_region *dax_region; >> - int rc = VM_FAULT_SIGBUS; >> phys_addr_t phys; >> pfn_t pfn; >> unsigned int fault_size = PAGE_SIZE; >> @@ -274,17 +274,11 @@ static int __dev_dax_pte_fault(struct dev_dax *dev_dax, struct vm_fault *vmf) >> >> pfn = phys_to_pfn_t(phys, dax_region->pfn_flags); >> >> - rc = vm_insert_mixed(vmf->vma, vmf->address, pfn); >> - >> - if (rc == -ENOMEM) >> - return VM_FAULT_OOM; >> - if (rc < 0 && rc != -EBUSY) >> - return VM_FAULT_SIGBUS; >> - >> - return VM_FAULT_NOPAGE; >> + return vmf_insert_mixed(vmf->vma, vmf->address, pfn); > > Ugh, so this change to vmf_insert_mixed() went upstream without fixing > the users? This changelog is now misleading as it does not mention > that is now an urgent standalone fix. On first read I assumed this was > part of a wider effort for 4.18. > > Grumble, we'll get this applied with a 'Fixes: 1c8f422059ae ("mm: > change return type to vm_fault_t")' tag. > Thanks for that explanation. The patch description is missing any kind of "why" (justification). -- ~Randy ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dax: Change return type to vm_fault_t 2018-04-16 16:29 ` Randy Dunlap @ 2018-04-16 16:38 ` Souptick Joarder 0 siblings, 0 replies; 13+ messages in thread From: Souptick Joarder @ 2018-04-16 16:38 UTC (permalink / raw) To: Randy Dunlap Cc: Dan Williams, linux-nvdimm, Matthew Wilcox, Linux MM, Andrew Morton, Michal Hocko On Mon, Apr 16, 2018 at 9:59 PM, Randy Dunlap <rdunlap@infradead.org> wrote: > On 04/16/2018 09:14 AM, Dan Williams wrote: >> On Sat, Apr 14, 2018 at 8:50 AM, Souptick Joarder <jrdr.linux@gmail.com> wrote: >>> Use new return type vm_fault_t for fault and >>> huge_fault handler. >>> >>> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com> >>> Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com> >>> --- >>> drivers/dax/device.c | 26 +++++++++++--------------- >>> 1 file changed, 11 insertions(+), 15 deletions(-) >>> >>> diff --git a/drivers/dax/device.c b/drivers/dax/device.c >>> index 2137dbc..a122701 100644 >>> --- a/drivers/dax/device.c >>> +++ b/drivers/dax/device.c >>> @@ -243,11 +243,11 @@ __weak phys_addr_t dax_pgoff_to_phys(struct dev_dax *dev_dax, pgoff_t pgoff, >>> return -1; >>> } >>> >>> -static int __dev_dax_pte_fault(struct dev_dax *dev_dax, struct vm_fault *vmf) >>> +static vm_fault_t __dev_dax_pte_fault(struct dev_dax *dev_dax, >>> + struct vm_fault *vmf) >>> { >>> struct device *dev = &dev_dax->dev; >>> struct dax_region *dax_region; >>> - int rc = VM_FAULT_SIGBUS; >>> phys_addr_t phys; >>> pfn_t pfn; >>> unsigned int fault_size = PAGE_SIZE; >>> @@ -274,17 +274,11 @@ static int __dev_dax_pte_fault(struct dev_dax *dev_dax, struct vm_fault *vmf) >>> >>> pfn = phys_to_pfn_t(phys, dax_region->pfn_flags); >>> >>> - rc = vm_insert_mixed(vmf->vma, vmf->address, pfn); >>> - >>> - if (rc == -ENOMEM) >>> - return VM_FAULT_OOM; >>> - if (rc < 0 && rc != -EBUSY) >>> - return VM_FAULT_SIGBUS; >>> - >>> - return VM_FAULT_NOPAGE; >>> + return vmf_insert_mixed(vmf->vma, vmf->address, pfn); >> >> Ugh, so this change to vmf_insert_mixed() went upstream without fixing >> the users? This changelog is now misleading as it does not mention >> that is now an urgent standalone fix. On first read I assumed this was >> part of a wider effort for 4.18. >> >> Grumble, we'll get this applied with a 'Fixes: 1c8f422059ae ("mm: >> change return type to vm_fault_t")' tag. >> > > Thanks for that explanation. The patch description is missing any kind > of "why" (justification). ok, I will send v2 with description. > > > -- > ~Randy ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dax: Change return type to vm_fault_t 2018-04-16 16:14 ` [PATCH] dax: Change return type to vm_fault_t Dan Williams 2018-04-16 16:29 ` Randy Dunlap @ 2018-04-16 17:47 ` Matthew Wilcox 2018-04-16 18:00 ` Dan Williams 2018-04-17 0:14 ` Theodore Y. Ts'o 2 siblings, 1 reply; 13+ messages in thread From: Matthew Wilcox @ 2018-04-16 17:47 UTC (permalink / raw) To: Dan Williams Cc: Souptick Joarder, linux-nvdimm, Linux MM, Andrew Morton, Michal Hocko On Mon, Apr 16, 2018 at 09:14:48AM -0700, Dan Williams wrote: > > - rc = vm_insert_mixed(vmf->vma, vmf->address, pfn); > > - > > - if (rc == -ENOMEM) > > - return VM_FAULT_OOM; > > - if (rc < 0 && rc != -EBUSY) > > - return VM_FAULT_SIGBUS; > > - > > - return VM_FAULT_NOPAGE; > > + return vmf_insert_mixed(vmf->vma, vmf->address, pfn); > > Ugh, so this change to vmf_insert_mixed() went upstream without fixing > the users? This changelog is now misleading as it does not mention > that is now an urgent standalone fix. On first read I assumed this was > part of a wider effort for 4.18. You read too quickly. vmf_insert_mixed() is a *new* function which *replaces* vm_insert_mixed() and awful-mangling-of-return-values-done-per-driver. Eventually vm_insert_mixed() will be deleted. But today is not that day. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dax: Change return type to vm_fault_t 2018-04-16 17:47 ` Matthew Wilcox @ 2018-04-16 18:00 ` Dan Williams 2018-04-16 18:21 ` Matthew Wilcox 0 siblings, 1 reply; 13+ messages in thread From: Dan Williams @ 2018-04-16 18:00 UTC (permalink / raw) To: Matthew Wilcox Cc: Souptick Joarder, linux-nvdimm, Linux MM, Andrew Morton, Michal Hocko On Mon, Apr 16, 2018 at 10:47 AM, Matthew Wilcox <willy@infradead.org> wrote: > On Mon, Apr 16, 2018 at 09:14:48AM -0700, Dan Williams wrote: >> > - rc = vm_insert_mixed(vmf->vma, vmf->address, pfn); >> > - >> > - if (rc == -ENOMEM) >> > - return VM_FAULT_OOM; >> > - if (rc < 0 && rc != -EBUSY) >> > - return VM_FAULT_SIGBUS; >> > - >> > - return VM_FAULT_NOPAGE; >> > + return vmf_insert_mixed(vmf->vma, vmf->address, pfn); >> >> Ugh, so this change to vmf_insert_mixed() went upstream without fixing >> the users? This changelog is now misleading as it does not mention >> that is now an urgent standalone fix. On first read I assumed this was >> part of a wider effort for 4.18. > > You read too quickly. vmf_insert_mixed() is a *new* function which > *replaces* vm_insert_mixed() and > awful-mangling-of-return-values-done-per-driver. > > Eventually vm_insert_mixed() will be deleted. But today is not that day. Ah, ok, thanks for the clarification. Then this patch should definitely be re-titled to "dax: convert to the new vmf_insert_mixed() helper". The vm_fault_t conversion is just a minor side-effect of that larger change. I assume this can wait for v4.18. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dax: Change return type to vm_fault_t 2018-04-16 18:00 ` Dan Williams @ 2018-04-16 18:21 ` Matthew Wilcox 2018-04-16 18:28 ` Souptick Joarder 0 siblings, 1 reply; 13+ messages in thread From: Matthew Wilcox @ 2018-04-16 18:21 UTC (permalink / raw) To: Dan Williams Cc: Souptick Joarder, linux-nvdimm, Linux MM, Andrew Morton, Michal Hocko On Mon, Apr 16, 2018 at 11:00:26AM -0700, Dan Williams wrote: > On Mon, Apr 16, 2018 at 10:47 AM, Matthew Wilcox <willy@infradead.org> wrote: > > On Mon, Apr 16, 2018 at 09:14:48AM -0700, Dan Williams wrote: > >> > - rc = vm_insert_mixed(vmf->vma, vmf->address, pfn); > >> > - > >> > - if (rc == -ENOMEM) > >> > - return VM_FAULT_OOM; > >> > - if (rc < 0 && rc != -EBUSY) > >> > - return VM_FAULT_SIGBUS; > >> > - > >> > - return VM_FAULT_NOPAGE; > >> > + return vmf_insert_mixed(vmf->vma, vmf->address, pfn); > >> > >> Ugh, so this change to vmf_insert_mixed() went upstream without fixing > >> the users? This changelog is now misleading as it does not mention > >> that is now an urgent standalone fix. On first read I assumed this was > >> part of a wider effort for 4.18. > > > > You read too quickly. vmf_insert_mixed() is a *new* function which > > *replaces* vm_insert_mixed() and > > awful-mangling-of-return-values-done-per-driver. > > > > Eventually vm_insert_mixed() will be deleted. But today is not that day. > > Ah, ok, thanks for the clarification. Then this patch should > definitely be re-titled to "dax: convert to the new vmf_insert_mixed() > helper". The vm_fault_t conversion is just a minor side-effect of that > larger change. I assume this can wait for v4.18. Yes, no particular hurry. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dax: Change return type to vm_fault_t 2018-04-16 18:21 ` Matthew Wilcox @ 2018-04-16 18:28 ` Souptick Joarder 2018-04-16 18:35 ` Dan Williams 0 siblings, 1 reply; 13+ messages in thread From: Souptick Joarder @ 2018-04-16 18:28 UTC (permalink / raw) To: Matthew Wilcox Cc: Dan Williams, linux-nvdimm, Linux MM, Andrew Morton, Michal Hocko On Mon, Apr 16, 2018 at 11:51 PM, Matthew Wilcox <willy@infradead.org> wrote: > On Mon, Apr 16, 2018 at 11:00:26AM -0700, Dan Williams wrote: >> On Mon, Apr 16, 2018 at 10:47 AM, Matthew Wilcox <willy@infradead.org> wrote: >> > On Mon, Apr 16, 2018 at 09:14:48AM -0700, Dan Williams wrote: >> >> > - rc = vm_insert_mixed(vmf->vma, vmf->address, pfn); >> >> > - >> >> > - if (rc == -ENOMEM) >> >> > - return VM_FAULT_OOM; >> >> > - if (rc < 0 && rc != -EBUSY) >> >> > - return VM_FAULT_SIGBUS; >> >> > - >> >> > - return VM_FAULT_NOPAGE; >> >> > + return vmf_insert_mixed(vmf->vma, vmf->address, pfn); >> >> >> >> Ugh, so this change to vmf_insert_mixed() went upstream without fixing >> >> the users? This changelog is now misleading as it does not mention >> >> that is now an urgent standalone fix. On first read I assumed this was >> >> part of a wider effort for 4.18. >> > >> > You read too quickly. vmf_insert_mixed() is a *new* function which >> > *replaces* vm_insert_mixed() and >> > awful-mangling-of-return-values-done-per-driver. >> > >> > Eventually vm_insert_mixed() will be deleted. But today is not that day. >> >> Ah, ok, thanks for the clarification. Then this patch should >> definitely be re-titled to "dax: convert to the new vmf_insert_mixed() >> helper". The vm_fault_t conversion is just a minor side-effect of that >> larger change. I assume this can wait for v4.18. The primary objective is to change the return type to vm_fault_t in all fault handlers and to support that we have replace vm_insert_mixed() with vmf_insert_ mixed() within one fault handler function. Do I really need to change the patch title ? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dax: Change return type to vm_fault_t 2018-04-16 18:28 ` Souptick Joarder @ 2018-04-16 18:35 ` Dan Williams 2018-04-16 18:37 ` Souptick Joarder 0 siblings, 1 reply; 13+ messages in thread From: Dan Williams @ 2018-04-16 18:35 UTC (permalink / raw) To: Souptick Joarder Cc: Matthew Wilcox, linux-nvdimm, Linux MM, Andrew Morton, Michal Hocko On Mon, Apr 16, 2018 at 11:28 AM, Souptick Joarder <jrdr.linux@gmail.com> wrote: > On Mon, Apr 16, 2018 at 11:51 PM, Matthew Wilcox <willy@infradead.org> wrote: >> On Mon, Apr 16, 2018 at 11:00:26AM -0700, Dan Williams wrote: >>> On Mon, Apr 16, 2018 at 10:47 AM, Matthew Wilcox <willy@infradead.org> wrote: >>> > On Mon, Apr 16, 2018 at 09:14:48AM -0700, Dan Williams wrote: >>> >> > - rc = vm_insert_mixed(vmf->vma, vmf->address, pfn); >>> >> > - >>> >> > - if (rc == -ENOMEM) >>> >> > - return VM_FAULT_OOM; >>> >> > - if (rc < 0 && rc != -EBUSY) >>> >> > - return VM_FAULT_SIGBUS; >>> >> > - >>> >> > - return VM_FAULT_NOPAGE; >>> >> > + return vmf_insert_mixed(vmf->vma, vmf->address, pfn); >>> >> >>> >> Ugh, so this change to vmf_insert_mixed() went upstream without fixing >>> >> the users? This changelog is now misleading as it does not mention >>> >> that is now an urgent standalone fix. On first read I assumed this was >>> >> part of a wider effort for 4.18. >>> > >>> > You read too quickly. vmf_insert_mixed() is a *new* function which >>> > *replaces* vm_insert_mixed() and >>> > awful-mangling-of-return-values-done-per-driver. >>> > >>> > Eventually vm_insert_mixed() will be deleted. But today is not that day. >>> >>> Ah, ok, thanks for the clarification. Then this patch should >>> definitely be re-titled to "dax: convert to the new vmf_insert_mixed() >>> helper". The vm_fault_t conversion is just a minor side-effect of that >>> larger change. I assume this can wait for v4.18. > > The primary objective is to change the return type to > vm_fault_t in all fault handlers and to support that > we have replace vm_insert_mixed() with vmf_insert_ > mixed() within one fault handler function. > > Do I really need to change the patch title ? At this point, yes, or at least mention the vm_insert_mixed --> vmf_insert_mixed conversion in the changelog. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dax: Change return type to vm_fault_t 2018-04-16 18:35 ` Dan Williams @ 2018-04-16 18:37 ` Souptick Joarder 2018-04-16 18:41 ` Randy Dunlap 0 siblings, 1 reply; 13+ messages in thread From: Souptick Joarder @ 2018-04-16 18:37 UTC (permalink / raw) To: Dan Williams Cc: Matthew Wilcox, linux-nvdimm, Linux MM, Andrew Morton, Michal Hocko On Tue, Apr 17, 2018 at 12:05 AM, Dan Williams <dan.j.williams@intel.com> wrote: > On Mon, Apr 16, 2018 at 11:28 AM, Souptick Joarder <jrdr.linux@gmail.com> wrote: >> On Mon, Apr 16, 2018 at 11:51 PM, Matthew Wilcox <willy@infradead.org> wrote: >>> On Mon, Apr 16, 2018 at 11:00:26AM -0700, Dan Williams wrote: >>>> On Mon, Apr 16, 2018 at 10:47 AM, Matthew Wilcox <willy@infradead.org> wrote: >>>> > On Mon, Apr 16, 2018 at 09:14:48AM -0700, Dan Williams wrote: >>>> >> > - rc = vm_insert_mixed(vmf->vma, vmf->address, pfn); >>>> >> > - >>>> >> > - if (rc == -ENOMEM) >>>> >> > - return VM_FAULT_OOM; >>>> >> > - if (rc < 0 && rc != -EBUSY) >>>> >> > - return VM_FAULT_SIGBUS; >>>> >> > - >>>> >> > - return VM_FAULT_NOPAGE; >>>> >> > + return vmf_insert_mixed(vmf->vma, vmf->address, pfn); >>>> >> >>>> >> Ugh, so this change to vmf_insert_mixed() went upstream without fixing >>>> >> the users? This changelog is now misleading as it does not mention >>>> >> that is now an urgent standalone fix. On first read I assumed this was >>>> >> part of a wider effort for 4.18. >>>> > >>>> > You read too quickly. vmf_insert_mixed() is a *new* function which >>>> > *replaces* vm_insert_mixed() and >>>> > awful-mangling-of-return-values-done-per-driver. >>>> > >>>> > Eventually vm_insert_mixed() will be deleted. But today is not that day. >>>> >>>> Ah, ok, thanks for the clarification. Then this patch should >>>> definitely be re-titled to "dax: convert to the new vmf_insert_mixed() >>>> helper". The vm_fault_t conversion is just a minor side-effect of that >>>> larger change. I assume this can wait for v4.18. >> >> The primary objective is to change the return type to >> vm_fault_t in all fault handlers and to support that >> we have replace vm_insert_mixed() with vmf_insert_ >> mixed() within one fault handler function. >> >> Do I really need to change the patch title ? > > At this point, yes, or at least mention the vm_insert_mixed --> > vmf_insert_mixed conversion in the changelog. Ok, I will add this in change log and send v2. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dax: Change return type to vm_fault_t 2018-04-16 18:37 ` Souptick Joarder @ 2018-04-16 18:41 ` Randy Dunlap 0 siblings, 0 replies; 13+ messages in thread From: Randy Dunlap @ 2018-04-16 18:41 UTC (permalink / raw) To: Souptick Joarder, Dan Williams Cc: Matthew Wilcox, linux-nvdimm, Linux MM, Andrew Morton, Michal Hocko On 04/16/2018 11:37 AM, Souptick Joarder wrote: > On Tue, Apr 17, 2018 at 12:05 AM, Dan Williams <dan.j.williams@intel.com> wrote: >> On Mon, Apr 16, 2018 at 11:28 AM, Souptick Joarder <jrdr.linux@gmail.com> wrote: >>> On Mon, Apr 16, 2018 at 11:51 PM, Matthew Wilcox <willy@infradead.org> wrote: >>>> On Mon, Apr 16, 2018 at 11:00:26AM -0700, Dan Williams wrote: >>>>> On Mon, Apr 16, 2018 at 10:47 AM, Matthew Wilcox <willy@infradead.org> wrote: >>>>>> On Mon, Apr 16, 2018 at 09:14:48AM -0700, Dan Williams wrote: >>>>>>>> - rc = vm_insert_mixed(vmf->vma, vmf->address, pfn); >>>>>>>> - >>>>>>>> - if (rc == -ENOMEM) >>>>>>>> - return VM_FAULT_OOM; >>>>>>>> - if (rc < 0 && rc != -EBUSY) >>>>>>>> - return VM_FAULT_SIGBUS; >>>>>>>> - >>>>>>>> - return VM_FAULT_NOPAGE; >>>>>>>> + return vmf_insert_mixed(vmf->vma, vmf->address, pfn); >>>>>>> >>>>>>> Ugh, so this change to vmf_insert_mixed() went upstream without fixing >>>>>>> the users? This changelog is now misleading as it does not mention >>>>>>> that is now an urgent standalone fix. On first read I assumed this was >>>>>>> part of a wider effort for 4.18. >>>>>> >>>>>> You read too quickly. vmf_insert_mixed() is a *new* function which >>>>>> *replaces* vm_insert_mixed() and >>>>>> awful-mangling-of-return-values-done-per-driver. >>>>>> >>>>>> Eventually vm_insert_mixed() will be deleted. But today is not that day. >>>>> >>>>> Ah, ok, thanks for the clarification. Then this patch should >>>>> definitely be re-titled to "dax: convert to the new vmf_insert_mixed() >>>>> helper". The vm_fault_t conversion is just a minor side-effect of that >>>>> larger change. I assume this can wait for v4.18. >>> >>> The primary objective is to change the return type to >>> vm_fault_t in all fault handlers and to support that >>> we have replace vm_insert_mixed() with vmf_insert_ >>> mixed() within one fault handler function. >>> >>> Do I really need to change the patch title ? >> >> At this point, yes, or at least mention the vm_insert_mixed --> >> vmf_insert_mixed conversion in the changelog. > > Ok, I will add this in change log and send v2. > and please try to use more columns per line of text. E.g. from the uio: patch: Use new return type vm_fault_t for fault handler in struct vm_operations_struct. For now, this is just documenting that the function returns a VM_ FAULT value rather than an errno. Once all inst ances are converted, vm_fault_t will become a di stinct type Several of those lines are chopped at odd places. -- ~Randy ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dax: Change return type to vm_fault_t 2018-04-16 16:14 ` [PATCH] dax: Change return type to vm_fault_t Dan Williams 2018-04-16 16:29 ` Randy Dunlap 2018-04-16 17:47 ` Matthew Wilcox @ 2018-04-17 0:14 ` Theodore Y. Ts'o 2018-04-17 0:19 ` Matthew Wilcox 2 siblings, 1 reply; 13+ messages in thread From: Theodore Y. Ts'o @ 2018-04-17 0:14 UTC (permalink / raw) To: Dan Williams Cc: Souptick Joarder, linux-nvdimm, Matthew Wilcox, Linux MM, Andrew Morton, Michal Hocko On Mon, Apr 16, 2018 at 09:14:48AM -0700, Dan Williams wrote: > Ugh, so this change to vmf_insert_mixed() went upstream without fixing > the users? This changelog is now misleading as it does not mention > that is now an urgent standalone fix. On first read I assumed this was > part of a wider effort for 4.18. Why is this an urgent fix? I thought all the return type change was did something completely innocuous that would not cause any real difference. Otherwise there are a dozen plus "fixups" to change the users that will now become urgent fixes, which I did *not* expect to be the case. (Where two are in ext2 and ext4, and where I planned to take my time and get them fixed in the next merge window, precisely becuase I did not *think* they were urgent.) - Ted ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dax: Change return type to vm_fault_t 2018-04-17 0:14 ` Theodore Y. Ts'o @ 2018-04-17 0:19 ` Matthew Wilcox 2018-04-17 4:08 ` Dan Williams 0 siblings, 1 reply; 13+ messages in thread From: Matthew Wilcox @ 2018-04-17 0:19 UTC (permalink / raw) To: Theodore Y. Ts'o Cc: Dan Williams, Souptick Joarder, linux-nvdimm, Linux MM, Andrew Morton, Michal Hocko On Mon, Apr 16, 2018 at 08:14:22PM -0400, Theodore Y. Ts'o wrote: > On Mon, Apr 16, 2018 at 09:14:48AM -0700, Dan Williams wrote: > > Ugh, so this change to vmf_insert_mixed() went upstream without fixing > > the users? This changelog is now misleading as it does not mention > > that is now an urgent standalone fix. On first read I assumed this was > > part of a wider effort for 4.18. > > Why is this an urgent fix? I thought all the return type change was > did something completely innocuous that would not cause any real > difference. Keep reading the thread; Dan is mistaken. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dax: Change return type to vm_fault_t 2018-04-17 0:19 ` Matthew Wilcox @ 2018-04-17 4:08 ` Dan Williams 0 siblings, 0 replies; 13+ messages in thread From: Dan Williams @ 2018-04-17 4:08 UTC (permalink / raw) To: Matthew Wilcox Cc: Theodore Y. Ts'o, Souptick Joarder, linux-nvdimm, Linux MM, Andrew Morton, Michal Hocko On Mon, Apr 16, 2018 at 5:19 PM, Matthew Wilcox <willy@infradead.org> wrote: > On Mon, Apr 16, 2018 at 08:14:22PM -0400, Theodore Y. Ts'o wrote: >> On Mon, Apr 16, 2018 at 09:14:48AM -0700, Dan Williams wrote: >> > Ugh, so this change to vmf_insert_mixed() went upstream without fixing >> > the users? This changelog is now misleading as it does not mention >> > that is now an urgent standalone fix. On first read I assumed this was >> > part of a wider effort for 4.18. >> >> Why is this an urgent fix? I thought all the return type change was >> did something completely innocuous that would not cause any real >> difference. > > Keep reading the thread; Dan is mistaken. Yes, false alarm, sorry. But we at least got a better changelog for the trouble. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-04-17 4:08 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20180414155059.GA18015@jordon-HP-15-Notebook-PC>
2018-04-16 16:14 ` [PATCH] dax: Change return type to vm_fault_t Dan Williams
2018-04-16 16:29 ` Randy Dunlap
2018-04-16 16:38 ` Souptick Joarder
2018-04-16 17:47 ` Matthew Wilcox
2018-04-16 18:00 ` Dan Williams
2018-04-16 18:21 ` Matthew Wilcox
2018-04-16 18:28 ` Souptick Joarder
2018-04-16 18:35 ` Dan Williams
2018-04-16 18:37 ` Souptick Joarder
2018-04-16 18:41 ` Randy Dunlap
2018-04-17 0:14 ` Theodore Y. Ts'o
2018-04-17 0:19 ` Matthew Wilcox
2018-04-17 4:08 ` Dan Williams
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox