| Message ID | 1603093393-12875-3-git-send-email-muneendra.kumar@broadcom.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | blkcg:Support to track FC storage blk io traffic | expand |
On Mon, Oct 19, 2020 at 01:12:57PM +0530, Muneendra wrote: > This Patch added a unique application identifier i.e > app_id knob to blkcg which allows identification of traffic > sources at an individual cgroup based Applications > (ex:virtual machine (VM))level in both host and > fabric infrastructure. > > Provided the interface blkcg_get_app_identifier to > grab the app identifier associated with a bio. > > Provided the interface blkcg_set_app_identifier to > set the app identifier in a blkcgrp associated with cgroup id > > Signed-off-by: Muneendra <muneendra.kumar@broadcom.com> > > --- > v2: > renamed app_identifier to app_id > removed the sysfs interface blkio.app_identifie under > /sys/fs/cgroup/blkio > Added a new interface blkcg_set_app_identifier > --- > block/blk-cgroup.c | 31 +++++++++++++++++++++++++++++++ > include/linux/blk-cgroup.h | 22 ++++++++++++++++++++++ > 2 files changed, 53 insertions(+) > > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c > index 619a79b51068..672971521010 100644 > --- a/block/blk-cgroup.c > +++ b/block/blk-cgroup.c > @@ -546,6 +546,37 @@ static struct blkcg_gq *blkg_lookup_check(struct blkcg *blkcg, > return __blkg_lookup(blkcg, q, true /* update_hint */); > } > > +/* > + * Sets the app_identifier field associted to blkcg > + * @buf: application identifier > + * @id: cgrp id > + * @len: size of appid > + */ > +int blkcg_set_app_identifier(char *buf, u64 id, size_t len) Can you please use a name which is more specific? > +{ > + struct cgroup *cgrp = NULL; > + struct cgroup_subsys_state *css = NULL; > + struct blkcg *blkcg = NULL; > + > + cgrp = cgroup_get_from_kernfs_id(id); > + if (!cgrp) > + return -ENOENT; > + > + css = cgroup_get_e_css(cgrp, &io_cgrp_subsys); > + if (!css) > + return -ENOENT; > + > + blkcg = css_to_blkcg(css); > + if (!blkcg) > + return -ENOENT; > + > + if (len > APPID_LEN) > + return -EINVAL; > + strlcpy(blkcg->app_id, buf, len); Shouldn't the cgrp and css be put before exit? > + return 0; > +} > +EXPORT_SYMBOL(blkcg_set_app_identifier); > + > /** > * blkg_conf_prep - parse and prepare for per-blkg config update > * @inputp: input string pointer > diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h > index c8fc9792ac77..5bd3f9f397ac 100644 > --- a/include/linux/blk-cgroup.h > +++ b/include/linux/blk-cgroup.h > @@ -30,6 +30,8 @@ > > /* Max limits for throttle policy */ > #define THROTL_IOPS_MAX UINT_MAX > +#define APPID_LEN 128 > + > > #ifdef CONFIG_BLK_CGROUP > > @@ -55,6 +57,8 @@ struct blkcg { > struct blkcg_policy_data *cpd[BLKCG_MAX_POLS]; > > struct list_head all_blkcgs_node; > + char app_id[APPID_LEN]; Please use a more specific name and gate them behind a CONFIG. Thanks.
Hi Tejun, Thanks for the input. >> + cgrp = cgroup_get_from_kernfs_id(id); >> + if (!cgrp) >> + return -ENOENT; >> + > >+ css = cgroup_get_e_css(cgrp, &io_cgrp_subsys); >> + if (!css) >> + return -ENOENT; >> + >> + blkcg = css_to_blkcg(css); >> + if (!blkcg) >> + return -ENOENT; > >+ return -EINVAL; > >+ strlcpy(blkcg->app_id, buf, len); >Shouldn't the cgrp and css be put before exit? [Muneendra]Correct me if iam wrong. You mean to add cgroup_put(cgrp) and css_put(css) before exit ? > + > > struct list_head all_blkcgs_node; > + char app_id[APPID_LEN]; >Please use a more specific name and gate them behind a CONFIG. [Muneendra] Agree will do the changes. Regards, Muneendra. Thanks. -- tejun
On Mon, Oct 19, 2020 at 08:43:05PM +0530, Muneendra Kumar M wrote: > Hi Tejun, > Thanks for the input. > >> + cgrp = cgroup_get_from_kernfs_id(id); > >> + if (!cgrp) > >> + return -ENOENT; > >> + > > >+ css = cgroup_get_e_css(cgrp, &io_cgrp_subsys); > >> + if (!css) > >> + return -ENOENT; > >> + > >> + blkcg = css_to_blkcg(css); > >> + if (!blkcg) > >> + return -ENOENT; > > >+ return -EINVAL; > > >+ strlcpy(blkcg->app_id, buf, len); > > >Shouldn't the cgrp and css be put before exit? > [Muneendra]Correct me if iam wrong. > You mean to add cgroup_put(cgrp) and css_put(css) before exit ? Yeah, as-is it'd leak references each time the function is called. Thanks.
Hi Tejun, Thanks for the input. I will add it my next version. Regards, Muneendra. -----Original Message----- From: Tejun Heo [mailto:htejun@gmail.com] On Behalf Of Tejun Heo Sent: Monday, October 19, 2020 8:50 PM To: Muneendra Kumar M <muneendra.kumar@broadcom.com> Cc: linux-block@vger.kernel.org; linux-scsi@vger.kernel.org; linux-nvme@lists.infradead.org; jsmart2021@gmail.com; emilne@redhat.com; mkumar@redhat.com; pbonzini@redhat.com Subject: Re: [RFC v2 02/18] blkcg: Added a app identifier support for blkcg On Mon, Oct 19, 2020 at 08:43:05PM +0530, Muneendra Kumar M wrote: > Hi Tejun, > Thanks for the input. > >> + cgrp = cgroup_get_from_kernfs_id(id); > >> + if (!cgrp) > >> + return -ENOENT; > >> + > > >+ css = cgroup_get_e_css(cgrp, &io_cgrp_subsys); > >> + if (!css) > >> + return -ENOENT; > >> + > >> + blkcg = css_to_blkcg(css); > >> + if (!blkcg) > >> + return -ENOENT; > > >+ return -EINVAL; > > >+ strlcpy(blkcg->app_id, buf, len); > > >Shouldn't the cgrp and css be put before exit? > [Muneendra]Correct me if iam wrong. > You mean to add cgroup_put(cgrp) and css_put(css) before exit ? Yeah, as-is it'd leak references each time the function is called. Thanks.
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 619a79b51068..672971521010 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -546,6 +546,37 @@ static struct blkcg_gq *blkg_lookup_check(struct blkcg *blkcg, return __blkg_lookup(blkcg, q, true /* update_hint */); } +/* + * Sets the app_identifier field associted to blkcg + * @buf: application identifier + * @id: cgrp id + * @len: size of appid + */ +int blkcg_set_app_identifier(char *buf, u64 id, size_t len) +{ + struct cgroup *cgrp = NULL; + struct cgroup_subsys_state *css = NULL; + struct blkcg *blkcg = NULL; + + cgrp = cgroup_get_from_kernfs_id(id); + if (!cgrp) + return -ENOENT; + + css = cgroup_get_e_css(cgrp, &io_cgrp_subsys); + if (!css) + return -ENOENT; + + blkcg = css_to_blkcg(css); + if (!blkcg) + return -ENOENT; + + if (len > APPID_LEN) + return -EINVAL; + strlcpy(blkcg->app_id, buf, len); + return 0; +} +EXPORT_SYMBOL(blkcg_set_app_identifier); + /** * blkg_conf_prep - parse and prepare for per-blkg config update * @inputp: input string pointer diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h index c8fc9792ac77..5bd3f9f397ac 100644 --- a/include/linux/blk-cgroup.h +++ b/include/linux/blk-cgroup.h @@ -30,6 +30,8 @@ /* Max limits for throttle policy */ #define THROTL_IOPS_MAX UINT_MAX +#define APPID_LEN 128 + #ifdef CONFIG_BLK_CGROUP @@ -55,6 +57,8 @@ struct blkcg { struct blkcg_policy_data *cpd[BLKCG_MAX_POLS]; struct list_head all_blkcgs_node; + char app_id[APPID_LEN]; + #ifdef CONFIG_CGROUP_WRITEBACK struct list_head cgwb_list; #endif @@ -206,6 +210,24 @@ struct gendisk *blkcg_conf_get_disk(char **inputp); int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol, char *input, struct blkg_conf_ctx *ctx); void blkg_conf_finish(struct blkg_conf_ctx *ctx); +int blkcg_set_app_identifier(char *buf, u64 id, size_t len); + +/** + * blkcg_get_app_identifier - grab the app identifier associated with a bio + * @bio: target bio + * + * This returns the app identifier associated with a bio, + * %NULL if not associated. + * Callers are expected to either handle %NULL or know association has been + * done prior to calling this. + */ +static inline char *blkcg_get_app_identifier(struct bio *bio) +{ + if (bio && bio->bi_blkg && + strlen(bio->bi_blkg->blkcg->app_id)) + return bio->bi_blkg->blkcg->app_id; + return NULL; +} /** * blkcg_css - find the current css
This Patch added a unique application identifier i.e app_id knob to blkcg which allows identification of traffic sources at an individual cgroup based Applications (ex:virtual machine (VM))level in both host and fabric infrastructure. Provided the interface blkcg_get_app_identifier to grab the app identifier associated with a bio. Provided the interface blkcg_set_app_identifier to set the app identifier in a blkcgrp associated with cgroup id Signed-off-by: Muneendra <muneendra.kumar@broadcom.com> --- v2: renamed app_identifier to app_id removed the sysfs interface blkio.app_identifie under /sys/fs/cgroup/blkio Added a new interface blkcg_set_app_identifier --- block/blk-cgroup.c | 31 +++++++++++++++++++++++++++++++ include/linux/blk-cgroup.h | 22 ++++++++++++++++++++++ 2 files changed, 53 insertions(+)