Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Message-ID: <2377bfb8-00bc-4863-8d70-425cb70c9c0b@intel.com>
Date:   Tue, 12 Dec 2023 09:54:38 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v6 1/3] x86/resctrl: Add mount option "mba_MBps_event"
Content-Language: en-US
To:     Peter Newman <peternewman@google.com>,
        Tony Luck <tony.luck@intel.com>
CC:     Fenghua Yu <fenghua.yu@intel.com>,
        Jonathan Corbet <corbet@lwn.net>,
        "Shuah Khan" <skhan@linuxfoundation.org>, <x86@kernel.org>,
        Shaopeng Tan <tan.shaopeng@fujitsu.com>,
        James Morse <james.morse@arm.com>,
        Jamie Iles <quic_jiles@quicinc.com>,
        Babu Moger <babu.moger@amd.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        <linux-kernel@vger.kernel.org>, <linux-doc@vger.kernel.org>,
        <patches@lists.linux.dev>
References: <20231201214737.104444-1-tony.luck@intel.com>
 <20231207195613.153980-1-tony.luck@intel.com>
 <20231207195613.153980-2-tony.luck@intel.com>
 <CALPaoCji1yzfkA=tms3LhYMvRB+wSJQM3qzPKrHNEa7a+KduTA@mail.gmail.com>
 <ZXORTTIUKWXOsd9p@agluck-desk3>
 <CALPaoCj_OvMzD8W9+jEORewwDquM6F-sOW32Ngcy+Gm6A1+OcQ@mail.gmail.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <CALPaoCj_OvMzD8W9+jEORewwDquM6F-sOW32Ngcy+Gm6A1+OcQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ejVIZHVSVEc1UTBZNTQ0dU8zMWlIUlVudG1OVU8xSk9UcmJyRjdmNzBsem1B?=
 =?utf-8?B?V2llNFp2UzhlL2wrbC9ZVzh6Q093eUpJWjFWMk9KRG5VazRxRzRsVjYwOWZQ?=
 =?utf-8?B?MnE4U1lnMXFmNmR5cmtCdC9vNExYZXI3Z2hjRUVCRjR4Z29OT0Vob0N0TytS?=
 =?utf-8?B?S29qWHJ4cmxBTGp6d0VPZGZDUWc2MHRqdVRqTVFWcUt4dWRHMHQ2WlpJZFE5?=
 =?utf-8?B?TFBGSkx6R0lTRWdRbzFPSWE2b0ZpVitTUHFzaG93WTFuckpScEV0c1lOZlpM?=
 =?utf-8?B?QnNyWlZQdG5wQWgrS1FPcFlPYmtSeWlhSDVsdlhoT3c4YnI4S3Z5OEk4VGd0?=
 =?utf-8?B?cEpTNHo1bHc3ZjdONE5ZcFpqWGRTT0pVOW02UThMV3dUUXUrY0xBY3gvSXZq?=
 =?utf-8?B?UUdFeXMzaUNESzRWRzhZUzRzYzJNUXlGVkNJeEw3Tk45Rll5WldoQmlTZTRp?=
 =?utf-8?B?NnFFaGNtd0UxeC9NYnVMUVRUaWZVZHJKZTBBb0JyMG12a2w4OFBvTGFoa0lB?=
 =?utf-8?B?Y0c2MCtqMzVtVzlsenZzbzc0dlJrSlBoKys3ekU5OXJzcWNzK3pnSDlFU3l3?=
 =?utf-8?B?SjQ0TW5EZVREVVYvYU5TNDI0UXJyaXhGeXI1d3phZzhhdi9VQUl2UVVMYS9J?=
 =?utf-8?B?dXZ3R1A3clNZZS8zQWpqWnJwS0xwY0lWaVp2Y2M3Vzd4OFZ6cnRLVmpNQ2x2?=
 =?utf-8?B?Q0xVMnYvaW9hVzNnZFlqbFpmb2F0S3ZsLzdVNFc4aXBKN216M0xuc1VSZGlr?=
 =?utf-8?B?VUJUVWdQNHprRDlzZEJNM1dybG9hZE1mZkhhc3ZyM0pIRW8xZDlFSU5kd2Zw?=
 =?utf-8?B?SmQ0L3NuS1Rybi92Z1E3YXRVWHcyOUVpNmU4d3F0cUtDOUord3lReU5nTVY1?=
 =?utf-8?B?WWQrTzByaWFFdGNTZ3BFNVRjNWVweXA2T01zZ2FvN2ZUVUpjRW5HTGhVZFF2?=
 =?utf-8?B?bDNOVXU4Y2xuNUk4ZWxNS3paQzMwUDJ5TXo3SnJVeTZJWlBtMHJycGxXV0RS?=
 =?utf-8?B?Ujc3Tm15NjU4YTEzV1hrcDlkcHdHL29PckJEL3R1blZYQnhObnFwcU1KRkVl?=
 =?utf-8?B?RUJvOFhiWldYdFl1Q3FtZ3hDL1Zkc0FtZE11WDdzT3ZMbmFsRTE1cUgwbWNR?=
 =?utf-8?B?YlNJdVFLdy9JaC9aU01rU2pMVERyV3pjWXM4L3lBblhjVGxtMmZ6bXovalFo?=
 =?utf-8?B?b09GSmx3bDZzWGMwZGZGN3FFRE9ha3FJbCtXL3IvRTNmbW9pTkluaS80OUUx?=
 =?utf-8?B?b0hjR2VrN0VmbVhGSnhEemhCYmo1VmlRVVRzcmxlU0VmSFd6bFlSV3BjQ25B?=
 =?utf-8?B?YTBXTEJHcHZ2WE95OXhRNkpOUzlpWUV2N0ovNmFXaTdDcnFEYTVRV2tTbWNS?=
 =?utf-8?B?SlVDelNNNW9lV3hOc3Fwdnp5cnJZSUlSYkdLWDNra1NyV2I0VXlQSnRoTnFt?=
 =?utf-8?B?ZFRKNnN3alo5RC9iMHI0TW5nNnA5R0R1MjFFaWhyZGhaa3pQa0c5d0ZGakJs?=
 =?utf-8?B?cmFaaUErNU9FeVpvbnNBS3A1UTFYU2xBYVJIWllkVHhXMHI5V2VnSkZEVW03?=
 =?utf-8?B?Z3huNGgyd0VaTVJmUGExNTFxTDdhU2RsazhRcnlWQ1VlQ2xrcWhuejNUYTdT?=
 =?utf-8?B?YjFVSjZtL25DSnBhL0RoakF5VzBuMUV5WWJieWt6cjZkcVByU1Z2QnJiWE9C?=
 =?utf-8?B?RkpDZ3ZOQms4bFFkOUpMQXo1enRHbFd6cmE4bndJaTBmQVB6OHBYRHJyUmtJ?=
 =?utf-8?B?d1p0L2pLLzhiQXI1dGdHRFdRWVpsaGZNaGo2VnRxSUxtY1pwMk9remdPMjk5?=
 =?utf-8?B?c2UxZkRoR1hGNU9ZMkVLUnBSMVA2OVRxK3M0RjRxckFENWxvZ3ZLNERuSnFG?=
 =?utf-8?B?TVFSb3BBd1MzclhtR3NGUkpTWEZpNU0wV0wzS1h6NXdYV3UzUnJSZmtvRk9V?=
 =?utf-8?B?SytWczFDNjZLYWdYUzBtNlU2d3BFaTlkaFFjUHpiWk90ZWRCQy9NckhkWm5N?=
 =?utf-8?B?eFkxMWV5b1hXUGpzckMxRmtsekF4eXpTWVVRQ05pcmFpVTR4UjV0SUg4Yktp?=
 =?utf-8?B?MTk4T3RpQ1E5TGVPNFJMMVgxYUtBcEplZkgvdWZ0TzNweGpDQmxOWGtZYnFw?=
 =?utf-8?B?WFBpM1NWREdhOXg3YzFaZVpRdUFXQ3pwZlhkdEg0VUdZb0hKVC9iTHEzVWIr?=
 =?utf-8?B?a2c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e685784a-4033-454b-ae4e-08dbfb3b69f7
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Dec 2023 17:54:40.3993
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: /MPvLj1YyFn5vpcOwlFBhW2aONa8GD1/hldRPj95Gg/GS3tfo/Nbg39rNa0Z9DzMPSOLRSvPHmWtH2NX6pCn/zLcM4wPB5y2P7PeafPP2fU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CYXPR11MB8712
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Tue, 12 Dec 2023 09:54:52 -0800 (PST)


On 12/8/2023 2:09 PM, Peter Newman wrote:
> On Fri, Dec 8, 2023 at 1:57 PM Tony Luck <tony.luck@intel.com> wrote:
>>
>> On Fri, Dec 08, 2023 at 10:17:08AM -0800, Peter Newman wrote:
>>> Hi Tony,
>>>
>>> On Thu, Dec 7, 2023 at 11:56 AM Tony Luck <tony.luck@intel.com> wrote:
>>>> @@ -2715,7 +2723,25 @@ static int rdt_parse_param(struct fs_context *fc, struct fs_parameter *param)
>>>>         case Opt_mba_mbps:
>>>>                 if (!supports_mba_mbps())
>>>>                         return -EINVAL;
>>>> -               ctx->enable_mba_mbps = true;
>>>> +               if (is_mbm_local_enabled())
>>>> +                       ctx->enable_mba_mbps_local = true;
>>>> +               else
>>>> +                       return -EINVAL;
>>>> +               return 0;
>>>> +       case Opt_mba_mbps_event:
>>>> +               if (!supports_mba_mbps())
>>>> +                       return -EINVAL;
>>>> +               if (!strcmp("mbm_local_bytes", param->string)) {
>>>> +                       if (!is_mbm_local_enabled())
>>>> +                               return -EINVAL;
>>>> +                       ctx->enable_mba_mbps_local = true;
>>>> +               } else if (!strcmp("mbm_total_bytes", param->string)) {
>>>> +                       if (!is_mbm_total_enabled())
>>>> +                               return -EINVAL;
>>>> +                       ctx->enable_mba_mbps_total = true;
>>>> +               } else {
>>>> +                       return -EINVAL;
>>>
>>> It looks like if I pass
>>> "mba_MBps_event=mbm_total_bytes,mba_MBps_event=mbm_local_bytes" I can
>>> set both flags true.
>>
>> That's going to be confusing. I'll add code to stop the user from
>> passing both options.
> 
> Also kind of confusing, after reading the second patch, I realized
> "mba_MBps_event=mbm_total_bytes,mba_MBps" also results in both being
> set. If you're able to fail the mount operation if both flags somehow
> get set, that would address this one too.

Are two separate flags required? All existing options within struct rdt_fs_context
are of type bool but that does not imply that it is the required type for
all. 

Reinette