Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1676939 > unrolled thread

Re: [PATCH V4 00/12] blktrace: output cgroup info

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V4 00/12] blktrace: output cgroup info Jens Axboe <axboe@kernel.dk> - 2017-06-28 20:00 +0200
    Re: [PATCH V4 00/12] blktrace: output cgroup info Tejun Heo <tj@kernel.org> - 2017-06-28 20:20 +0200
      Re: [PATCH V4 00/12] blktrace: output cgroup info Jens Axboe <axboe@kernel.dk> - 2017-06-28 23:00 +0200
        Re: [PATCH V4 00/12] blktrace: output cgroup info Tejun Heo <tj@kernel.org> - 2017-06-28 23:30 +0200
          Re: [PATCH V4 00/12] blktrace: output cgroup info Greg KH <gregkh@linuxfoundation.org> - 2017-06-29 15:00 +0200
        Re: [PATCH V4 00/12] blktrace: output cgroup info Shaohua Li <shli@kernel.org> - 2017-06-29 20:40 +0200

#1676939 — Re: [PATCH V4 00/12] blktrace: output cgroup info

On 06/28/2017 10:53 AM, Shaohua Li wrote:
> On Wed, Jun 28, 2017 at 10:43:48AM -0600, Jens Axboe wrote:
>> On 06/28/2017 10:29 AM, Shaohua Li wrote:
>>> From: Shaohua Li <shli@fb.com>
>>>
>>> Hi,
>>>
>>> Currently blktrace isn't cgroup aware. blktrace prints out task name of current
>>> context, but the task of current context isn't always in the cgroup where the
>>> BIO comes from. We can't use task name to find out IO cgroup. For example,
>>> Writeback BIOs always comes from flusher thread but the BIOs are for different
>>> blk cgroups. Request could be requeued and dispatched from completely different
>>> tasks. MD/DM are another examples. This brings challenges if we want to use
>>> blktrace for performance tunning with cgroup enabled.
>>>
>>> This patchset try to fix the gap. We print out cgroup fhandle info in blktrace.
>>> Userspace can use open_by_handle_at() syscall to find the cgroup by fhandle. Or
>>> userspace can use name_to_handle_at() syscall to find fhandle for a cgroup and
>>> use a BPF program to filter out blktrace for a specific cgroup.
>>>
>>> The first 6 patches adds export operation handlers for kernfs, so userspace can
>>> use open_by_handle_at/name_to_handle_at to a kernfs file. Later patches make
>>> blktrace output cgroup info.
>>
>> Series looks fine to me. I don't know how you want to split or funnel it,
>> since it touches multiple different parts. Would it make sense to split this
>> series into two - one for the kernfs changes, and then a subsequent block
>> series that depend on that?
> 
> What's the best practice to do this without building errors? Ask Tejun
> to merge the first 7 patches first?

Yes, and then resend the block patches, just noting that dependency. Then
we can funnel them in like that.

-- 
Jens Axboe

[toc] | [next] | [standalone]

#1677041

Hello,

On Wed, Jun 28, 2017 at 10:54:28AM -0600, Jens Axboe wrote:
> >> Series looks fine to me. I don't know how you want to split or funnel it,
> >> since it touches multiple different parts. Would it make sense to split this
> >> series into two - one for the kernfs changes, and then a subsequent block
> >> series that depend on that?
> > 
> > What's the best practice to do this without building errors? Ask Tejun
> > to merge the first 7 patches first?
> 
> Yes, and then resend the block patches, just noting that dependency. Then
> we can funnel them in like that.

I wonder whether it'd be a lot easier to route the whole series
through one tree, most likely block.  Greg, would that be okay with
you?  Alternatively, we can route the whole thing through driver tree
if Jens is okay with that.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1677167

On 06/28/2017 12:11 PM, Tejun Heo wrote:
> Hello,
> 
> On Wed, Jun 28, 2017 at 10:54:28AM -0600, Jens Axboe wrote:
>>>> Series looks fine to me. I don't know how you want to split or funnel it,
>>>> since it touches multiple different parts. Would it make sense to split this
>>>> series into two - one for the kernfs changes, and then a subsequent block
>>>> series that depend on that?
>>>
>>> What's the best practice to do this without building errors? Ask Tejun
>>> to merge the first 7 patches first?
>>
>> Yes, and then resend the block patches, just noting that dependency. Then
>> we can funnel them in like that.
> 
> I wonder whether it'd be a lot easier to route the whole series
> through one tree, most likely block.  Greg, would that be okay with
> you?  Alternatively, we can route the whole thing through driver tree
> if Jens is okay with that.

Personally I don't care that much, but the risk of conflicts is much
higher on the block side, than on the kernfs side. So might be the
path of less resistance to pull it through the block tree. And I'd be
happy to do that, if the sign offs on the kernfs side are sufficient.

-- 
Jens Axboe

[toc] | [prev] | [next] | [standalone]

#1677193

Hello,

On Wed, Jun 28, 2017 at 02:57:38PM -0600, Jens Axboe wrote:
> Personally I don't care that much, but the risk of conflicts is much
> higher on the block side, than on the kernfs side. So might be the
> path of less resistance to pull it through the block tree. And I'd be
> happy to do that, if the sign offs on the kernfs side are sufficient.

Block tree it is then.  Let's wait for Greg to respond.

Thanks.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1677721

On Wed, Jun 28, 2017 at 05:25:25PM -0400, Tejun Heo wrote:
> Hello,
> 
> On Wed, Jun 28, 2017 at 02:57:38PM -0600, Jens Axboe wrote:
> > Personally I don't care that much, but the risk of conflicts is much
> > higher on the block side, than on the kernfs side. So might be the
> > path of less resistance to pull it through the block tree. And I'd be
> > happy to do that, if the sign offs on the kernfs side are sufficient.
> 
> Block tree it is then.  Let's wait for Greg to respond.

block tree is fine with me, I'll go ack the kernfs patches now.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1678056

On Wed, Jun 28, 2017 at 02:57:38PM -0600, Jens Axboe wrote:
> On 06/28/2017 12:11 PM, Tejun Heo wrote:
> > Hello,
> > 
> > On Wed, Jun 28, 2017 at 10:54:28AM -0600, Jens Axboe wrote:
> >>>> Series looks fine to me. I don't know how you want to split or funnel it,
> >>>> since it touches multiple different parts. Would it make sense to split this
> >>>> series into two - one for the kernfs changes, and then a subsequent block
> >>>> series that depend on that?
> >>>
> >>> What's the best practice to do this without building errors? Ask Tejun
> >>> to merge the first 7 patches first?
> >>
> >> Yes, and then resend the block patches, just noting that dependency. Then
> >> we can funnel them in like that.
> > 
> > I wonder whether it'd be a lot easier to route the whole series
> > through one tree, most likely block.  Greg, would that be okay with
> > you?  Alternatively, we can route the whole thing through driver tree
> > if Jens is okay with that.
> 
> Personally I don't care that much, but the risk of conflicts is much
> higher on the block side, than on the kernfs side. So might be the
> path of less resistance to pull it through the block tree. And I'd be
> happy to do that, if the sign offs on the kernfs side are sufficient.

Jens,
Now we have the stamps, could you please queue the patches in your tree?

For the patch 10, please drop it right now. With Christoph's integrity patch,
we only need to free cgroup info in bio_endio. I'll resend a patch.

Thanks,
Shaohua

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web