Re: [PATCH v13 02/18] EDAC: Add scrub control feature

Borislav Petkov
2024-10-23T16:26:06Z

On Wed, Oct 23, 2024 at 04:04:05PM +0000, Shiju Jose wrote:
> >Why do you need a separate "enable" flag?
> >
> >Why can't it be: "writing into "addr" starts the on-demand scrubbing"?
> If 'enable' attribute is removed , then there is an ordering with setting address + size.

No, there won't be. You clarify the ordering and if someone doesn't adhere to
it, you check for 0 values and return.

> Also user space can't check whether scrubbing is enabled or not.

That one is semi-valid. You can set addr to 0 when scrubbing is done but then
userspace might wanna know which address it scrubbed.

> >What are those three good for and why are they exposed?
> Scrub has an overhead when running and that may want to be reduced by
> just taking longer to do it.
> Min and max scrub cycle duration returns the range of scrub rate
> supported by the device.

This *definitely* needs to be part of the documentation explaining the API.

> >I think fail. What is a scrub feature good for if it doesn't have ops?
> Here continue to check any other feature (for eg. ECS, memory repair or another scrub instance) listed
> by the parent device in the ras_features[].

Why would you tolerate a semi-broken feature?

This is all open source code. People will fix it when they test their feature
which is missing ops. There's no point in allowing any of that.

Btw, do me a favor, pls, and trim your mails when you reply just like I do.
You don't want to leave text quoted to which you are not replying to.

Thx.

--
Regards/Gruss,
Boris.