All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: Candidate for late revert ?
       [not found]   ` <yq138fm98in.fsf@sermon.lab.mkp.net>
@ 2014-06-03  0:45     ` Tejun Heo
  2014-06-03  1:11       ` Martin K. Petersen
  0 siblings, 1 reply; 4+ messages in thread
From: Tejun Heo @ 2014-06-03  0:45 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: Linus Torvalds, Alan Cox, linux-ide

Hello, Martin.

(cc'ing linux-ide)

On Mon, Jun 02, 2014 at 07:49:52PM -0400, Martin K. Petersen wrote:
> I'm starting to think we'll have to make queued trim an opt-in for
> now. Every vendor appears to screw it up. I had a third patch in my
> series that would allow the users to override the type of trim in
> sysfs. It was mainly written as an aid to the SSD vendors so they could
> test queued trim on drives we had blacklisted. I didn't post it because
> I was afraid people would shoot themselves in the foot. But maybe opt-in
> for enthusiasts is a better approach in the short term...

The problem with that approach is that it's very likely that we won't
be able to take advantage of that feature ever.  IIRC, we did a
similar thing with FUA support.  We added the support, a bunch of
controllers and devices were broken, we hid it behind a module option
with the goal being eventually developing a whitelist.  Of course,
there's no way to actually find out which combinations work once the
feature is disabled by default and libata FUA support is essentially
dead, which BTW might be the right conclusion.  It's not like FUA
makes noticeable difference and quite likely that vendors haven't
gotten around to fix the issues anyway and won't ever.

The one difference is that, unlike FUA, queued trim is actually
useful, so I kinda wanna give it more fighting chances before we
declare defeat and bury it behind a module option.  That said, I
wouldn't have any problem with applying blacklist liberally for
devices which show any sign of issues.  If there isn't no definite
confirmation from the vendor regarding what the issue was and which
firmware fixes it, I think blacklisting the whole family of devices
isn't a bad idea.  If this feature is gonna survive, given its actual
usefulness, I think / hope it would, the trade-off of blacklisting
existing devices widely makes pretty good sense if that'd allow us to
salvage the feature in the long term.

So, let's keep blacklisting liberally for now.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Candidate for late revert ?
  2014-06-03  0:45     ` Candidate for late revert ? Tejun Heo
@ 2014-06-03  1:11       ` Martin K. Petersen
  2014-06-03  1:15         ` Tejun Heo
  0 siblings, 1 reply; 4+ messages in thread
From: Martin K. Petersen @ 2014-06-03  1:11 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Martin K. Petersen, Linus Torvalds, Alan Cox, linux-ide

>>>>> "Tejun" == Tejun Heo <tj@kernel.org> writes:

Tejun> The problem with that approach is that it's very likely that we
Tejun> won't be able to take advantage of that feature ever.

Well, maybe. Ubuntu just turned on discard by default (for better and
worse). Took a while to get here with unqueued trim but it doesn't look
like they've had too much fallout.

The problem with queued trim is that we're in the bootstrap phase,
vendors generally don't have anything to test it with. The ones I talk
to claim that it's not really working in Windows yet. So we're the only
ones that actually support it.

Tejun> That said, I wouldn't have any problem with applying blacklist
Tejun> liberally for devices which show any sign of issues.  If there
Tejun> isn't no definite confirmation from the vendor regarding what the
Tejun> issue was and which firmware fixes it, I think blacklisting the
Tejun> whole family of devices isn't a bad idea.

Yeah. The fun thing in this case is that Micron specifically issued this
firmware release to fix queued trim. And they did test with our code. As
did I. And yet something broke for users.

Tejun> So, let's keep blacklisting liberally for now.

OK. And how do you feel about making a disable/unqueued/queued sysfs
knob? I'd really like to make it easy for the drive vendors to test. And
if we liberally blacklist their firmware releases then we have
significantly raised the barrier for their testing. Because all of a
sudden they need to start mucking with kernel builds instead of booting
the latest $DISTRO Live CD.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Candidate for late revert ?
  2014-06-03  1:11       ` Martin K. Petersen
@ 2014-06-03  1:15         ` Tejun Heo
  2014-06-03  1:17           ` Martin K. Petersen
  0 siblings, 1 reply; 4+ messages in thread
From: Tejun Heo @ 2014-06-03  1:15 UTC (permalink / raw)
  To: Martin K. Petersen; +Cc: Linus Torvalds, Alan Cox, linux-ide

Hello,

On Mon, Jun 02, 2014 at 09:11:52PM -0400, Martin K. Petersen wrote:
> OK. And how do you feel about making a disable/unqueued/queued sysfs
> knob? I'd really like to make it easy for the drive vendors to test. And
> if we liberally blacklist their firmware releases then we have
> significantly raised the barrier for their testing. Because all of a
> sudden they need to start mucking with kernel builds instead of booting
> the latest $DISTRO Live CD.

I have no problem with adding debug features as long as it's clear
that it's a debug feature - please gate it with a module param which
clearly indicates that it's for debugging.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Candidate for late revert ?
  2014-06-03  1:15         ` Tejun Heo
@ 2014-06-03  1:17           ` Martin K. Petersen
  0 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2014-06-03  1:17 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Martin K. Petersen, Linus Torvalds, Alan Cox, linux-ide

>>>>> "Tejun" == Tejun Heo <tj@kernel.org> writes:

Tejun> I have no problem with adding debug features as long as it's
Tejun> clear that it's a debug feature - please gate it with a module
Tejun> param which clearly indicates that it's for debugging.

OK, will do.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-06-03  1:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20140603000728.28fca8fe@www.etchedpixels.co.uk>
     [not found] ` <CA+55aFykTbMRKku1bDd_wHCjyD+Yf2v-B8-c+NKMZ0AnNkpVrw@mail.gmail.com>
     [not found]   ` <yq138fm98in.fsf@sermon.lab.mkp.net>
2014-06-03  0:45     ` Candidate for late revert ? Tejun Heo
2014-06-03  1:11       ` Martin K. Petersen
2014-06-03  1:15         ` Tejun Heo
2014-06-03  1:17           ` Martin K. Petersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.