Hi all,
Today's linux-next merge of the tip tree got a conflict in:
arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
between commit:
58e4e43911f8 ("kernfs, sysfs, cgroup, intel_rdt: Support fs_context")
from the vfs tree and commit:
a3dbd01e6c9d ("x86/intel_rdt: Create character device exposing pseudo-locked region")
from the tip tree.
I fixed it up (I think - see below) and can carry the fix as necessary.
This is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 38a54f56ff40,7b4a09d81a30..000000000000
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@@ -1270,9 -1861,16 +1842,15 @@@ static int rdt_get_tree(struct fs_conte
rdtgroup_default.mon.mon_data_kn = kn_mondata;
}
+ ret = rdt_pseudo_lock_init();
+ if (ret) {
+ dentry = ERR_PTR(ret);
+ goto out_mondata;
+ }
+
- dentry = kernfs_mount(fs_type, flags, rdt_root,
- RDTGROUP_SUPER_MAGIC, NULL);
- if (IS_ERR(dentry))
+ ret = kernfs_get_tree(fc);
+ if (ret < 0)
- goto out_mondata;
+ goto out_psl;
if (rdt_alloc_capable)
static_branch_enable_cpuslocked(&rdt_alloc_enable_key);
Hi all,
On Fri, 22 Jun 2018 11:53:46 +1000 Stephen Rothwell <[email protected]> wrote:
>
> diff --cc arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> index 38a54f56ff40,7b4a09d81a30..000000000000
> --- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> @@@ -1270,9 -1861,16 +1842,15 @@@ static int rdt_get_tree(struct fs_conte
> rdtgroup_default.mon.mon_data_kn = kn_mondata;
> }
>
> + ret = rdt_pseudo_lock_init();
> + if (ret) {
> + dentry = ERR_PTR(ret);
Actually I have removed the above line as well.
--
Cheers,
Stephen Rothwell
All,
On 6/21/2018 6:53 PM, Stephen Rothwell wrote:
> Today's linux-next merge of the tip tree got a conflict in:
>
> arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
>
> between commit:
>
> 58e4e43911f8 ("kernfs, sysfs, cgroup, intel_rdt: Support fs_context")
>
> from the vfs tree and commit:
>
> a3dbd01e6c9d ("x86/intel_rdt: Create character device exposing pseudo-loc=
> ked region")
>
> from the tip tree.
>
> I fixed it up (I think - see below) and can carry the fix as necessary.
> This is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> index 38a54f56ff40,7b4a09d81a30..000000000000
> --- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> @@@ -1270,9 -1861,16 +1842,15 @@@ static int rdt_get_tree(struct fs_conte
> rdtgroup_default.mon.mon_data_kn = kn_mondata;
> }
>
> + ret = rdt_pseudo_lock_init();
> + if (ret) {
> + dentry = ERR_PTR(ret);
> + goto out_mondata;
> + }
> +
> - dentry = kernfs_mount(fs_type, flags, rdt_root,
> - RDTGROUP_SUPER_MAGIC, NULL);
> - if (IS_ERR(dentry))
> + ret = kernfs_get_tree(fc);
> + if (ret < 0)
> - goto out_mondata;
> + goto out_psl;
>
> if (rdt_alloc_capable)
> static_branch_enable_cpuslocked(&rdt_alloc_enable_key);
>> + ret = rdt_pseudo_lock_init();
>> + if (ret) {
>> + dentry = ERR_PTR(ret);
>
> Actually I have removed the above line as well.
I am the contributor of the pseudo-locking related code that caused the
conflict.
Stephen, thank you very much for proceeding with the merge and fixing it
up. I copied your later fix addition to this email and combined they
look good. I did some basic smoke testing of the merged code and it works.
Thomas and David, please let me know what I can do from my side to help
with this.
As summary, with the above changes combined the merge can be done using:
->8-
diff --cc arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 7b4a09d81a30,38a54f56ff40..caa2754f8948
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@@ -1861,16 -1270,9 +1842,13 @@@ static int rdt_get_tree(struct fs_conte
rdtgroup_default.mon.mon_data_kn = kn_mondata;
}
+ ret = rdt_pseudo_lock_init();
- if (ret) {
- dentry = ERR_PTR(ret);
++ if (ret)
+ goto out_mondata;
- }
+
- dentry = kernfs_mount(fs_type, flags, rdt_root,
- RDTGROUP_SUPER_MAGIC, NULL);
- if (IS_ERR(dentry))
+ ret = kernfs_get_tree(fc);
+ if (ret < 0)
- goto out_mondata;
+ goto out_psl;
if (rdt_alloc_capable)
static_branch_enable_cpuslocked(&rdt_alloc_enable_key);
Reinette
On Thu, 21 Jun 2018, Reinette Chatre wrote:
> I am the contributor of the pseudo-locking related code that caused the
> conflict.
>
> Stephen, thank you very much for proceeding with the merge and fixing it
> up. I copied your later fix addition to this email and combined they
> look good. I did some basic smoke testing of the merged code and it works.
>
> Thomas and David, please let me know what I can do from my side to help
> with this.
I'll have a look later today.
>
> As summary, with the above changes combined the merge can be done using:
>
> ->8-
> diff --cc arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> index 7b4a09d81a30,38a54f56ff40..caa2754f8948
> --- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> +++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
> @@@ -1861,16 -1270,9 +1842,13 @@@ static int rdt_get_tree(struct fs_conte
> rdtgroup_default.mon.mon_data_kn = kn_mondata;
> }
>
> + ret = rdt_pseudo_lock_init();
> - if (ret) {
> - dentry = ERR_PTR(ret);
> ++ if (ret)
> + goto out_mondata;
> - }
> +
> - dentry = kernfs_mount(fs_type, flags, rdt_root,
> - RDTGROUP_SUPER_MAGIC, NULL);
> - if (IS_ERR(dentry))
> + ret = kernfs_get_tree(fc);
> + if (ret < 0)
> - goto out_mondata;
> + goto out_psl;
>
> if (rdt_alloc_capable)
> static_branch_enable_cpuslocked(&rdt_alloc_enable_key);
>
> Reinette
>
>
Reinette Chatre <[email protected]> wrote:
> Thomas and David, please let me know what I can do from my side to help
> with this.
You could try basing on Al Viro's for-next tree which has the mount API
changes in it. Sorry, I was hoping that this would go in the last merge
window, but I don't think Al has sufficient bandwidth available.
Note that it's likely to get rebased shortly as I have some patches to apply,
which given we're not at -rc2 yet, I'd like to roll into the original patches
to for bisectability reasons.
Also, Eric has some user_ns fixes that conflict with my procfs changes that he
wants to get upstream, though Linus said no - so I'm not sure what's going to
happen there.
David
On Fri, Jun 22, 2018 at 01:45:23PM +0100, David Howells wrote:
> Reinette Chatre <[email protected]> wrote:
>
> > Thomas and David, please let me know what I can do from my side to help
> > with this.
>
> You could try basing on Al Viro's for-next tree which has the mount API
> changes in it.
Umm... That would be a massive headache for everyone involved; the changes
in there have very little in common with what you are doing in rdt_mount(),
so it might make sense to start with a minimal never-rebased branch that
would
* define rdt_pseudo_lock_init as 0
* define rdt_pseudo_lock_release as empty
* do the rdt_mount() part of a3dbd01e6c9d
* have commit message along the lines of
"hooks in rdt_mount() for rdt_pseudo_lock to use
Functionally a no-op right now; the only reason for having that
as a never-rebased branch to get rdt_pseudo_lock and mount series
out of each other's hair"
Base that on -rc1, then pull it into your rdt branch and David could pull the
same into his.
On Fri, 22 Jun 2018, Al Viro wrote:
> On Fri, Jun 22, 2018 at 01:45:23PM +0100, David Howells wrote:
> > Reinette Chatre <[email protected]> wrote:
> >
> > > Thomas and David, please let me know what I can do from my side to help
> > > with this.
> >
> > You could try basing on Al Viro's for-next tree which has the mount API
> > changes in it.
>
> Umm... That would be a massive headache for everyone involved; the changes
> in there have very little in common with what you are doing in rdt_mount(),
> so it might make sense to start with a minimal never-rebased branch that
> would
> * define rdt_pseudo_lock_init as 0
> * define rdt_pseudo_lock_release as empty
> * do the rdt_mount() part of a3dbd01e6c9d
> * have commit message along the lines of
> "hooks in rdt_mount() for rdt_pseudo_lock to use
>
> Functionally a no-op right now; the only reason for having that
> as a never-rebased branch to get rdt_pseudo_lock and mount series
> out of each other's hair"
>
> Base that on -rc1, then pull it into your rdt branch and David could pull the
> same into his.
Yes, that works.
Reinette, can you please look into creating that ordering. Then we just zap
the existing branch and redo it with this scheme.
Thanks,
tglx
Hi Thomas,
On 6/22/2018 6:39 AM, Thomas Gleixner wrote:
> On Fri, 22 Jun 2018, Al Viro wrote:
>> On Fri, Jun 22, 2018 at 01:45:23PM +0100, David Howells wrote:
>>> Reinette Chatre <[email protected]> wrote:
>>>
>>>> Thomas and David, please let me know what I can do from my side to help
>>>> with this.
>>>
>>> You could try basing on Al Viro's for-next tree which has the mount API
>>> changes in it.
>>
>> Umm... That would be a massive headache for everyone involved; the changes
>> in there have very little in common with what you are doing in rdt_mount(),
>> so it might make sense to start with a minimal never-rebased branch that
>> would
>> * define rdt_pseudo_lock_init as 0
>> * define rdt_pseudo_lock_release as empty
>> * do the rdt_mount() part of a3dbd01e6c9d
>> * have commit message along the lines of
>> "hooks in rdt_mount() for rdt_pseudo_lock to use
>>
>> Functionally a no-op right now; the only reason for having that
>> as a never-rebased branch to get rdt_pseudo_lock and mount series
>> out of each other's hair"
>>
>> Base that on -rc1, then pull it into your rdt branch and David could pull the
>> same into his.
>
> Yes, that works.
>
> Reinette, can you please look into creating that ordering. Then we just zap
> the existing branch and redo it with this scheme.
Will do. How would you prefer to consume this to make the branches
simple to create? Is it ok if I create a new patch series with Al's
suggestion above as the first commit?
The original pseudo-locking patch series consisted out of two sections
with the pseudo-locking specific parts starting in the middle. If I
create a new series with the above change then it will not be cleanly
separate anymore. Is that ok?
Reinette
On Fri, 22 Jun 2018, Reinette Chatre wrote:
> On 6/22/2018 6:39 AM, Thomas Gleixner wrote:
> > On Fri, 22 Jun 2018, Al Viro wrote:
> >> On Fri, Jun 22, 2018 at 01:45:23PM +0100, David Howells wrote:
> >>> Reinette Chatre <[email protected]> wrote:
> >>>
> >>>> Thomas and David, please let me know what I can do from my side to help
> >>>> with this.
> >>>
> >>> You could try basing on Al Viro's for-next tree which has the mount API
> >>> changes in it.
> >>
> >> Umm... That would be a massive headache for everyone involved; the changes
> >> in there have very little in common with what you are doing in rdt_mount(),
> >> so it might make sense to start with a minimal never-rebased branch that
> >> would
> >> * define rdt_pseudo_lock_init as 0
> >> * define rdt_pseudo_lock_release as empty
> >> * do the rdt_mount() part of a3dbd01e6c9d
> >> * have commit message along the lines of
> >> "hooks in rdt_mount() for rdt_pseudo_lock to use
> >>
> >> Functionally a no-op right now; the only reason for having that
> >> as a never-rebased branch to get rdt_pseudo_lock and mount series
> >> out of each other's hair"
> >>
> >> Base that on -rc1, then pull it into your rdt branch and David could pull the
> >> same into his.
> >
> > Yes, that works.
> >
> > Reinette, can you please look into creating that ordering. Then we just zap
> > the existing branch and redo it with this scheme.
>
> Will do. How would you prefer to consume this to make the branches
> simple to create? Is it ok if I create a new patch series with Al's
> suggestion above as the first commit?
>
> The original pseudo-locking patch series consisted out of two sections
> with the pseudo-locking specific parts starting in the middle. If I
> create a new series with the above change then it will not be cleanly
> separate anymore. Is that ok?
That's fine. Just mention it clearly in the changelog of that first one or
two patches you need for that.
Thanks,
tglx