* [PATCH v2] MAINTAINERS: consolidate vm-event/monitor entry @ 2023-08-31 6:15 Jan Beulich 2023-09-06 12:40 ` Anthony PERARD 0 siblings, 1 reply; 5+ messages in thread From: Jan Beulich @ 2023-08-31 6:15 UTC (permalink / raw) To: xen-devel@lists.xenproject.org Cc: Tamas K Lengyel, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu If the F: description is to be trusted, the two xen/arch/x86/hvm/ lines were fully redundant with the earlier wildcard ones. Arch header files, otoh, were no longer covered by anything as of the move from include/asm-*/ to arch/*/include/asm/. Further also generalize (by folding) the x86- and Arm-specific mem_access.c entries. Finally, again assuming the F: description can be trusted, there's no point listing arch/, common/, and include/ entries separately. Fold them all. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v2: Further fold patterns. --- Triggered by me looking at the entry in the context of Oleksii's RISC-V preparatory patch. --- a/MAINTAINERS +++ b/MAINTAINERS @@ -558,20 +558,9 @@ R: Alexandru Isaila <aisaila@bitdefender R: Petre Pircalabu <ppircalabu@bitdefender.com> S: Supported F: tools/misc/xen-access.c -F: xen/arch/*/monitor.c -F: xen/arch/*/vm_event.c -F: xen/arch/arm/mem_access.c -F: xen/arch/x86/include/asm/hvm/monitor.h -F: xen/arch/x86/include/asm/hvm/vm_event.h -F: xen/arch/x86/mm/mem_access.c -F: xen/arch/x86/hvm/monitor.c -F: xen/arch/x86/hvm/vm_event.c -F: xen/common/mem_access.c -F: xen/common/monitor.c -F: xen/common/vm_event.c -F: xen/include/*/mem_access.h -F: xen/include/*/monitor.h -F: xen/include/*/vm_event.h +F: xen/*/mem_access.[ch] +F: xen/*/monitor.[ch] +F: xen/*/vm_event.[ch] VPCI M: Roger Pau Monné <roger.pau@citrix.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] MAINTAINERS: consolidate vm-event/monitor entry 2023-08-31 6:15 [PATCH v2] MAINTAINERS: consolidate vm-event/monitor entry Jan Beulich @ 2023-09-06 12:40 ` Anthony PERARD 2023-09-06 12:50 ` Jan Beulich 0 siblings, 1 reply; 5+ messages in thread From: Anthony PERARD @ 2023-09-06 12:40 UTC (permalink / raw) To: Jan Beulich Cc: xen-devel@lists.xenproject.org, Tamas K Lengyel, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu On Thu, Aug 31, 2023 at 08:15:13AM +0200, Jan Beulich wrote: > If the F: description is to be trusted, the two xen/arch/x86/hvm/ > lines were fully redundant with the earlier wildcard ones. Arch header > files, otoh, were no longer covered by anything as of the move from > include/asm-*/ to arch/*/include/asm/. Further also generalize (by > folding) the x86- and Arm-specific mem_access.c entries. > > Finally, again assuming the F: description can be trusted, there's no > point listing arch/, common/, and include/ entries separately. Fold > them all. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > -F: xen/arch/*/monitor.c > -F: xen/arch/*/vm_event.c > -F: xen/arch/arm/mem_access.c > -F: xen/arch/x86/include/asm/hvm/monitor.h > -F: xen/arch/x86/include/asm/hvm/vm_event.h > -F: xen/arch/x86/mm/mem_access.c > -F: xen/arch/x86/hvm/monitor.c > -F: xen/arch/x86/hvm/vm_event.c > -F: xen/common/mem_access.c > -F: xen/common/monitor.c > -F: xen/common/vm_event.c > -F: xen/include/*/mem_access.h > -F: xen/include/*/monitor.h > -F: xen/include/*/vm_event.h > +F: xen/*/mem_access.[ch] > +F: xen/*/monitor.[ch] > +F: xen/*/vm_event.[ch] Hi, Did you mean to for example change the maintainer ship of "xen/arch/x86/mm/mem_access.c"? Before it was: - VM EVENT, MEM ACCESS and MONITOR - X86 MEMORY MANAGEMENT - X86 ARCHITECTURE And now, it's just: - X86 MEMORY MANAGEMENT - X86 ARCHITECTURE (see ./scripts/get_maintainer.pl --sections -f xen/arch/x86/mm/mem_access.c) Also, now "xen/include/xen/monitor.h" is only "THE REST". On the other hand, there's no change for "xen/common/monitor.c", so the pattern works for this particular file. Cheers, -- Anthony PERARD ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] MAINTAINERS: consolidate vm-event/monitor entry 2023-09-06 12:40 ` Anthony PERARD @ 2023-09-06 12:50 ` Jan Beulich 2023-09-06 14:45 ` Anthony PERARD 0 siblings, 1 reply; 5+ messages in thread From: Jan Beulich @ 2023-09-06 12:50 UTC (permalink / raw) To: Anthony PERARD Cc: xen-devel@lists.xenproject.org, Tamas K Lengyel, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu On 06.09.2023 14:40, Anthony PERARD wrote: > On Thu, Aug 31, 2023 at 08:15:13AM +0200, Jan Beulich wrote: >> If the F: description is to be trusted, the two xen/arch/x86/hvm/ >> lines were fully redundant with the earlier wildcard ones. Arch header >> files, otoh, were no longer covered by anything as of the move from >> include/asm-*/ to arch/*/include/asm/. Further also generalize (by >> folding) the x86- and Arm-specific mem_access.c entries. >> >> Finally, again assuming the F: description can be trusted, there's no >> point listing arch/, common/, and include/ entries separately. Fold >> them all. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> -F: xen/arch/*/monitor.c >> -F: xen/arch/*/vm_event.c >> -F: xen/arch/arm/mem_access.c >> -F: xen/arch/x86/include/asm/hvm/monitor.h >> -F: xen/arch/x86/include/asm/hvm/vm_event.h >> -F: xen/arch/x86/mm/mem_access.c >> -F: xen/arch/x86/hvm/monitor.c >> -F: xen/arch/x86/hvm/vm_event.c >> -F: xen/common/mem_access.c >> -F: xen/common/monitor.c >> -F: xen/common/vm_event.c >> -F: xen/include/*/mem_access.h >> -F: xen/include/*/monitor.h >> -F: xen/include/*/vm_event.h >> +F: xen/*/mem_access.[ch] >> +F: xen/*/monitor.[ch] >> +F: xen/*/vm_event.[ch] > > > Hi, > > Did you mean to for example change the maintainer ship of > "xen/arch/x86/mm/mem_access.c"? Before it was: > - VM EVENT, MEM ACCESS and MONITOR > - X86 MEMORY MANAGEMENT > - X86 ARCHITECTURE > And now, it's just: > - X86 MEMORY MANAGEMENT > - X86 ARCHITECTURE > > (see ./scripts/get_maintainer.pl --sections -f xen/arch/x86/mm/mem_access.c) > > Also, now "xen/include/xen/monitor.h" is only "THE REST". No, no change of maintainership was intended. But there was an uncertainty, which is why I said "assuming the F: description can be trusted". So ... > On the other hand, there's no change for "xen/common/monitor.c", so the > pattern works for this particular file. ... together with this observation, I take it that F: */net/* all files in "any top level directory"/net is actually at best misleading / ambiguous - I read it as not just a single level of directories, but it may well be that that's what is meant. At which point the question is how "any number of directories" could be expressed. Would **/ or .../**/... work here? I'm afraid my Perl is far from sufficient to actually spot where (and hence how) this is handled in the script. Jan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] MAINTAINERS: consolidate vm-event/monitor entry 2023-09-06 12:50 ` Jan Beulich @ 2023-09-06 14:45 ` Anthony PERARD 2023-09-06 15:32 ` Jan Beulich 0 siblings, 1 reply; 5+ messages in thread From: Anthony PERARD @ 2023-09-06 14:45 UTC (permalink / raw) To: Jan Beulich Cc: xen-devel@lists.xenproject.org, Tamas K Lengyel, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu On Wed, Sep 06, 2023 at 02:50:22PM +0200, Jan Beulich wrote: > On 06.09.2023 14:40, Anthony PERARD wrote: > > On Thu, Aug 31, 2023 at 08:15:13AM +0200, Jan Beulich wrote: > >> If the F: description is to be trusted, the two xen/arch/x86/hvm/ > >> lines were fully redundant with the earlier wildcard ones. Arch header > >> files, otoh, were no longer covered by anything as of the move from > >> include/asm-*/ to arch/*/include/asm/. Further also generalize (by > >> folding) the x86- and Arm-specific mem_access.c entries. > >> > >> Finally, again assuming the F: description can be trusted, there's no > >> point listing arch/, common/, and include/ entries separately. Fold > >> them all. > >> > >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > >> --- > >> -F: xen/arch/*/monitor.c > >> -F: xen/arch/*/vm_event.c > >> -F: xen/arch/arm/mem_access.c > >> -F: xen/arch/x86/include/asm/hvm/monitor.h > >> -F: xen/arch/x86/include/asm/hvm/vm_event.h > >> -F: xen/arch/x86/mm/mem_access.c > >> -F: xen/arch/x86/hvm/monitor.c > >> -F: xen/arch/x86/hvm/vm_event.c > >> -F: xen/common/mem_access.c > >> -F: xen/common/monitor.c > >> -F: xen/common/vm_event.c > >> -F: xen/include/*/mem_access.h > >> -F: xen/include/*/monitor.h > >> -F: xen/include/*/vm_event.h > >> +F: xen/*/mem_access.[ch] > >> +F: xen/*/monitor.[ch] > >> +F: xen/*/vm_event.[ch] > > > > > > Hi, > > > > Did you mean to for example change the maintainer ship of > > "xen/arch/x86/mm/mem_access.c"? Before it was: > > - VM EVENT, MEM ACCESS and MONITOR > > - X86 MEMORY MANAGEMENT > > - X86 ARCHITECTURE > > And now, it's just: > > - X86 MEMORY MANAGEMENT > > - X86 ARCHITECTURE > > > > (see ./scripts/get_maintainer.pl --sections -f xen/arch/x86/mm/mem_access.c) > > > > Also, now "xen/include/xen/monitor.h" is only "THE REST". > > No, no change of maintainership was intended. But there was an uncertainty, > which is why I said "assuming the F: description can be trusted". So ... > > > On the other hand, there's no change for "xen/common/monitor.c", so the > > pattern works for this particular file. > > ... together with this observation, I take it that > > F: */net/* all files in "any top level directory"/net > > is actually at best misleading / ambiguous - I read it as not just a single > level of directories, but it may well be that that's what is meant. At I guess the ambiguity would lie in the word "files". Here, "files" is a single file and not a directory, unlike the shell globing which would include directories with a '*'. The first '*' is described at "any top level directory", but it is also "only top level directory". This kind of tells me that there is only a single level of directories that is match by '*'. > which point the question is how "any number of directories" could be > expressed. Would **/ or .../**/... work here? I'm afraid my Perl is far > from sufficient to actually spot where (and hence how) this is handled in > the script. I think you could write a regexp with the "N:" type instead of "F:". This is described Linux's MAINTAINERS file, but not ours, yet our get_maintainer.pl script has the functionality. It might be nice to be able to write just '**' but until someone implement that, we could go for a regex, which is more complicated and more prone to mistake. So I think in the short-term, you want: N: ^xen/.*/mem_access\.[ch]$ N: ^xen/.*/monitor\.[ch]$ N: ^xen/.*/vm_event\.[ch]$ As for adding "**", there's maybe something to do with "file_match_pattern()" in get_maintainer.pl, this function compare the number of '/' in both the pattern and the filepath to find out if a '*' only match one level of directory or more. Cheers, -- Anthony PERARD ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] MAINTAINERS: consolidate vm-event/monitor entry 2023-09-06 14:45 ` Anthony PERARD @ 2023-09-06 15:32 ` Jan Beulich 0 siblings, 0 replies; 5+ messages in thread From: Jan Beulich @ 2023-09-06 15:32 UTC (permalink / raw) To: Anthony PERARD Cc: xen-devel@lists.xenproject.org, Tamas K Lengyel, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu On 06.09.2023 16:45, Anthony PERARD wrote: > On Wed, Sep 06, 2023 at 02:50:22PM +0200, Jan Beulich wrote: >> On 06.09.2023 14:40, Anthony PERARD wrote: >>> On Thu, Aug 31, 2023 at 08:15:13AM +0200, Jan Beulich wrote: >>>> If the F: description is to be trusted, the two xen/arch/x86/hvm/ >>>> lines were fully redundant with the earlier wildcard ones. Arch header >>>> files, otoh, were no longer covered by anything as of the move from >>>> include/asm-*/ to arch/*/include/asm/. Further also generalize (by >>>> folding) the x86- and Arm-specific mem_access.c entries. >>>> >>>> Finally, again assuming the F: description can be trusted, there's no >>>> point listing arch/, common/, and include/ entries separately. Fold >>>> them all. >>>> >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>>> --- >>>> -F: xen/arch/*/monitor.c >>>> -F: xen/arch/*/vm_event.c >>>> -F: xen/arch/arm/mem_access.c >>>> -F: xen/arch/x86/include/asm/hvm/monitor.h >>>> -F: xen/arch/x86/include/asm/hvm/vm_event.h >>>> -F: xen/arch/x86/mm/mem_access.c >>>> -F: xen/arch/x86/hvm/monitor.c >>>> -F: xen/arch/x86/hvm/vm_event.c >>>> -F: xen/common/mem_access.c >>>> -F: xen/common/monitor.c >>>> -F: xen/common/vm_event.c >>>> -F: xen/include/*/mem_access.h >>>> -F: xen/include/*/monitor.h >>>> -F: xen/include/*/vm_event.h >>>> +F: xen/*/mem_access.[ch] >>>> +F: xen/*/monitor.[ch] >>>> +F: xen/*/vm_event.[ch] >>> >>> >>> Hi, >>> >>> Did you mean to for example change the maintainer ship of >>> "xen/arch/x86/mm/mem_access.c"? Before it was: >>> - VM EVENT, MEM ACCESS and MONITOR >>> - X86 MEMORY MANAGEMENT >>> - X86 ARCHITECTURE >>> And now, it's just: >>> - X86 MEMORY MANAGEMENT >>> - X86 ARCHITECTURE >>> >>> (see ./scripts/get_maintainer.pl --sections -f xen/arch/x86/mm/mem_access.c) >>> >>> Also, now "xen/include/xen/monitor.h" is only "THE REST". >> >> No, no change of maintainership was intended. But there was an uncertainty, >> which is why I said "assuming the F: description can be trusted". So ... >> >>> On the other hand, there's no change for "xen/common/monitor.c", so the >>> pattern works for this particular file. >> >> ... together with this observation, I take it that >> >> F: */net/* all files in "any top level directory"/net >> >> is actually at best misleading / ambiguous - I read it as not just a single >> level of directories, but it may well be that that's what is meant. At > > I guess the ambiguity would lie in the word "files". Here, "files" is a > single file and not a directory, unlike the shell globing which would > include directories with a '*'. > > The first '*' is described at "any top level directory", but it is also > "only top level directory". This kind of tells me that there is only a > single level of directories that is match by '*'. > >> which point the question is how "any number of directories" could be >> expressed. Would **/ or .../**/... work here? I'm afraid my Perl is far >> from sufficient to actually spot where (and hence how) this is handled in >> the script. > > I think you could write a regexp with the "N:" type instead of "F:". > This is described Linux's MAINTAINERS file, but not ours, yet our > get_maintainer.pl script has the functionality. It might be nice to be > able to write just '**' but until someone implement that, we could go > for a regex, which is more complicated and more prone to mistake. > > So I think in the short-term, you want: > > N: ^xen/.*/mem_access\.[ch]$ > N: ^xen/.*/monitor\.[ch]$ > N: ^xen/.*/vm_event\.[ch]$ Except that as per Linux'es description F: and N: differ beyond how file names are expressed / matched. I don't think I want to be the one to introduce something which I've been complaining about on the Linux side (people being Cc-ed just because at some point they ended up touching a file). I conclude that for the time being I ought to revert that change of mine. Jan > As for adding "**", there's maybe something to do with > "file_match_pattern()" in get_maintainer.pl, this function compare the > number of '/' in both the pattern and the filepath to find out if a '*' > only match one level of directory or more. > > Cheers, > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-09-06 15:32 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-31 6:15 [PATCH v2] MAINTAINERS: consolidate vm-event/monitor entry Jan Beulich 2023-09-06 12:40 ` Anthony PERARD 2023-09-06 12:50 ` Jan Beulich 2023-09-06 14:45 ` Anthony PERARD 2023-09-06 15:32 ` Jan Beulich
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.