Hi Tony, On 12/4/23 12:16, Tony Luck wrote: > On Mon, Dec 04, 2023 at 10:24:58AM -0600, Moger, Babu wrote: >> Hi Tony, >> >> You are intending to achieve two things at once here. >> 1. Adding new mount option >> 2. Changing behaviour for the current option. >> I think you need to split this patch into two. Few comments below. > > Hi Babu, > > Thanks for looking at this patch. > > You are right. I will split the patch into two as you suggest. > >> On 12/1/23 15:47, Tony Luck wrote: >>> The MBA Software Controller(mba_sc) is a feedback loop that uses >>> measurements of local memory bandwidth to adjust MBA throttling levels to >>> keep workloads in a resctrl group within a target bandwidth set in the >>> schemata file. >>> >>> But on Intel systems the memory bandwidth monitoring events are >>> independently enumerated. It is possible for a system to support >>> total memory bandwidth monitoring, but not support local bandwidth >>> monitoring. On such a system a user could not enable mba_sc mode. >>> Users will see this highly unhelpful error message from mount: >>> >>> # mount -t resctrl -o mba_MBps resctrl /sys/fs/resctrl >>> mount: /sys/fs/resctrl: wrong fs type, bad option, bad superblock on >>> resctrl, missing codepage or helper program, or other error. >>> dmesg(1) may have more information after failed mount system call. >>> >>> dmesg(1) does not provide any additional information. >>> >>> Add a new mount option "mba_MBps_event=[local|total]" that allows >>> a user to specify which monitoring event to use. Also modify the >>> existing "mba_MBps" option to switch to total bandwidth monitoring >>> if local monitoring is not available. >> >> I am not sure why you need both these options. I feel you just need one of >> these options. > > I should have included "changes since v4" in with this message, and > pasted in some parts of this earlier messge from the discussion about > v4: > > https://lore.kernel.org/all/ZWpF5m4mIeZdK8kv@agluck-desk3/ > > Having the option take "local" would give a way for a user to > avoid the failover to using "total" if they really didn't want > that to happen. Yes. I saw the thread. Even then I feel having two similar options can cause confusion. I feel it is enough just to solve the original problem. Giving more options to a corner cases is a overkill in my opinion. Thanks Babu > > Not in that message, because I didn't think of it until later, it > opens the door for different events in the future. > > But I'm also open to other suggestions on naming and function of > mount options here. > > Thanks > > -Tony -- Thanks Babu Moger
