| Message ID | 20090810192809.GA16800@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
Michael S. Tsirkin wrote: > devices should have the final say over which virtio features they > support. E.g. indirect entries may or may not make sense in the context > of virtio-console. Move the common bits from virtio-pci to an inline > function and let each device call it. > What drove this in vhost? Normally, the common features are transport features and the devices should have absolutely no knowledge of transport feature (since they're transport dependent). IOW, VIRTIO_RING_F_INDIRECT_DESC is meaningless to virtio-console because virtio-console has no idea what the ring implementation is that it sits on top of. Regards, Anthony Liguori
On Mon, Aug 10, 2009 at 02:37:20PM -0500, Anthony Liguori wrote: > Michael S. Tsirkin wrote: >> devices should have the final say over which virtio features they >> support. E.g. indirect entries may or may not make sense in the context >> of virtio-console. Move the common bits from virtio-pci to an inline >> function and let each device call it. >> > > What drove this in vhost? vhost does not support indirect ring entries and notify on empty yet. > Normally, the common features are transport features and the devices > should have absolutely no knowledge of transport feature (since they're > transport dependent). Good point. But 1. note that with my patch they don't. They call virtio_get_common_features and that's all. 2. some features may not make sense for some devices. For example, it is quite possible that indirect ring entries feature improves performance on block but hurts on net, as net has a similar merged buffers feature. Someone should try benchmarking with it disabled, and it becomes possible with my patch. > IOW, VIRTIO_RING_F_INDIRECT_DESC is meaningless to virtio-console > because virtio-console has no idea what the ring implementation is that > it sits on top of. At some level. Same can be said to be true for virtio-pci: it sits below the ring implementation. However, it is not *completely* meaningless: some devices may benefit from indirect entries, others may not, or may benefit from disabling them. > Regards, > > Anthony Liguori >
Michael S. Tsirkin wrote: >> Normally, the common features are transport features and the devices >> should have absolutely no knowledge of transport feature (since they're >> transport dependent). >> > > Good point. But > > 1. note that with my patch they don't. They call > virtio_get_common_features and that's all. > > 2. some features may not make sense for some devices. For example, it is > quite possible that indirect ring entries feature improves performance > on block but hurts on net, as net has a similar merged buffers feature. > Someone should try benchmarking with it disabled, and it becomes > possible with my patch. > I don't necessarily disagree but I think your patch goes about it the wrong way. There ought to be a way to layer qdev properties that achieves this goal so that when you create a virtio-pci-block device, you have the ability to turn off indirect sg without virtio-block having to know what that is. For your use-case, I wonder if you're integrating at the wrong level. If you implement a ring in-kernel, maybe the thing to do is introduce more layering in qemu like we have in the kernel so that you can easily add a new ring backend type. At any rate, see if you can achieve the same goal with qdev properties. If you could, you should be able to hack something up easily to disable this for vhost without completely overhauling qemu's virtio implementation. Regards, Anthony Liguori
On Mon, Aug 10, 2009 at 03:33:59PM -0500, Anthony Liguori wrote: > Michael S. Tsirkin wrote: >>> Normally, the common features are transport features and the devices >>> should have absolutely no knowledge of transport feature (since >>> they're transport dependent). >>> >> >> Good point. But >> >> 1. note that with my patch they don't. They call >> virtio_get_common_features and that's all. >> >> 2. some features may not make sense for some devices. For example, it is >> quite possible that indirect ring entries feature improves performance >> on block but hurts on net, as net has a similar merged buffers feature. >> Someone should try benchmarking with it disabled, and it becomes >> possible with my patch. >> > > I don't necessarily disagree but I think your patch goes about it the > wrong way. > > There ought to be a way to layer qdev properties that achieves this goal > so that when you create a virtio-pci-block device, you have the ability > to turn off indirect sg without virtio-block having to know what that is. I don't understand, sorry. Why do you insist on involving pci here? ring layout has nothing to do with pci, does it? With my patch, virtio-block does not know what indirect sg is. It just does enable/disable. virtio net has device-specific feature that overlaps with indirect entries. So insisting that devices should just ignore transport does not make sense, to me. > For your use-case, I wonder if you're integrating at the wrong level. Forget about that for now. Let's solve the generic problem. > Regards, > > Anthony Liguori
Michael S. Tsirkin wrote: > On Mon, Aug 10, 2009 at 03:33:59PM -0500, Anthony Liguori wrote: > >> There ought to be a way to layer qdev properties that achieves this goal >> so that when you create a virtio-pci-block device, you have the ability >> to turn off indirect sg without virtio-block having to know what that is. >> > > I don't understand, sorry. Why do you insist on involving pci here? > ring layout has nothing to do with pci, does it? What I'm saying is that virtio-blk-pci, which is the qdev instantiation of virtio-pci + virtio-blk, should be able to have a set of qdev properties that is composed of a combination of at least two sets of properties: virtio-blk's qdev properties and virtio-pci's qdev properties. Right now, all of the properties are defined in virtio-pci.c, so you could add a property that was DEFINE_PROP_BOOL("indirect-sg", ...), that you could then use to selectively enable/disable indirect sg on virtio-blk-pci devices without ever having to involve virtio-blk.c. Ideally, we wouldn't have the properties centralized in virtio-pci.c. Rather, it would be nice if virtio-blk.c could have a set of properties and virtio-pci.c could just add those properties to it's own set of properties. Today, we don't have a concept of a ring abstraction. If we did, then virtio-ring.c could have it's own set of properties. N.B. I expect that the in-kernel virtio-net device is going to be separate qdev device than virtio-net-pci. It can have an identical guest interface but within qemu, it should be a separate device. This is how we handle the in-kernel PIT and it's how we should handle the in-kernel APIC. Regards, Anthony Liguori
On Mon, Aug 10, 2009 at 05:35:13PM -0500, Anthony Liguori wrote: > Michael S. Tsirkin wrote: >> On Mon, Aug 10, 2009 at 03:33:59PM -0500, Anthony Liguori wrote: >> >>> There ought to be a way to layer qdev properties that achieves this >>> goal so that when you create a virtio-pci-block device, you have the >>> ability to turn off indirect sg without virtio-block having to know >>> what that is. >>> >> >> I don't understand, sorry. Why do you insist on involving pci here? >> ring layout has nothing to do with pci, does it? > > What I'm saying is that virtio-blk-pci, which is the qdev instantiation > of virtio-pci + virtio-blk, should be able to have a set of qdev > properties that is composed of a combination of at least two sets of > properties: virtio-blk's qdev properties and virtio-pci's qdev > properties. Yea. But indirect ring entries is not virtio-pci property. And even with virtio-pci properies, such as MSI, specific device should have control over number of vectors. > Right now, all of the properties are defined in virtio-pci.c, so you > could add a property that was DEFINE_PROP_BOOL("indirect-sg", ...), that > you could then use to selectively enable/disable indirect sg on > virtio-blk-pci devices without ever having to involve virtio-blk.c. Me as a user? We can't expect the user to tweak such low level stuff for each device. So devices such as block should have a say in which ring format options are used, in a way optimal for the specific usage. My example is that virtio net has merged buffers so indirect ring is probably just useless. And again, pci seems to have nothing to do with it, so why drag it in? > Ideally, we wouldn't have the properties centralized in virtio-pci.c. > Rather, it would be nice if virtio-blk.c could have a set of properties > and virtio-pci.c could just add those properties to it's own set of > properties. That's what my patch did. think of feature bits as properties. The set of properties can not currently be controlled by user, but this can be added if there is need. > Today, we don't have a concept of a ring abstraction. If we did, then > virtio-ring.c could have it's own set of properties. Yes but devices might and do care about which of these properties get used. > N.B. I expect that the in-kernel virtio-net device is going to be > separate qdev device than virtio-net-pci. It can have an identical > guest interface but within qemu, it should be a separate device. This > is how we handle the in-kernel PIT and it's how we should handle the > in-kernel APIC. Ugh. What advantages does this have? This would break things like migrating between userspace and kernel virtio (something that I support). IMO, this should work like MSI, detect capability and just have virtio go faster, with a disable flag for troubleshooting purposes. Can migration between in-kernel and userspace PIT work? If yes we would be better off changing that, as well. Separate devices should be for things that have guest-visible differences. Don't try to encode random information into the device name. > Regards, > > Anthony Liguori
Michael S. Tsirkin wrote: > On Mon, Aug 10, 2009 at 05:35:13PM -0500, Anthony Liguori wrote: > >> What I'm saying is that virtio-blk-pci, which is the qdev instantiation >> of virtio-pci + virtio-blk, should be able to have a set of qdev >> properties that is composed of a combination of at least two sets of >> properties: virtio-blk's qdev properties and virtio-pci's qdev >> properties. >> > > Yea. But indirect ring entries is not virtio-pci property. It's a ring feature and the ring implementation is currently in the generic virtio code. Ring features really have no home today so virtio-pci seems logical. > And ev > with virtio-pci properies, such as MSI, specific device should > have control over number of vectors. > Devices, or instantiation of the devices? The later is what I'm suggesting. Let's say we supported virtio-vbus along with virtio-pci. What does virtio_blk_get_features() do to mask out sg_indirect? For all virtio-blk knows, it could be on top of virtio-vbus. > Me as a user? We can't expect the user to tweak such low level stuff for > each device. So devices such as block should have a say in which ring > format options are used, in a way optimal for the specific usage. My > example is that virtio net has merged buffers so indirect ring is > probably just useless. And again, pci seems to have nothing to do with > it, so why drag it in? > If you want to tweak such thing, why not use default property values for virtio-blk-pci's definition in virtio-pci.c? That keeps it out of virtio-blk.c. >> separate qdev device than virtio-net-pci. It can have an identical >> guest interface but within qemu, it should be a separate device. This >> is how we handle the in-kernel PIT and it's how we should handle the >> in-kernel APIC. >> > > Ugh. What advantages does this have? It keeps a clean separate of the two devices. It actually ends up making things a lot easier to understand because it's clear what portions of code are not being used for the in-kernel device models. > This would break things like > migrating between userspace and kernel virtio (something that I > support). The PIT uses a common state structure and common code for save/restore. This makes migration compatible. > IMO, this should work like MSI, detect capability and just > have virtio go faster, with a disable flag for troubleshooting purposes. > > Can migration between in-kernel and userspace PIT work? > If yes we would be better off changing that, as well. > Take a look at i8524{,-kvm.c} in qemu-kvm and how it's instantiated in pc.c. It ends up being really straight forward. > Separate devices should be for things that have guest-visible > differences. Don't try to encode random information into the device > name. > In this case, it's two separate implementations of the same device. I think it makes sense for them to be separate devices. Regards, Anthony Liguori
On Tue, Aug 11, 2009 at 08:15:23AM -0500, Anthony Liguori wrote: > Michael S. Tsirkin wrote: >> On Mon, Aug 10, 2009 at 05:35:13PM -0500, Anthony Liguori wrote: >> >>> What I'm saying is that virtio-blk-pci, which is the qdev >>> instantiation of virtio-pci + virtio-blk, should be able to have a >>> set of qdev properties that is composed of a combination of at least >>> two sets of properties: virtio-blk's qdev properties and >>> virtio-pci's qdev properties. >>> >> >> Yea. But indirect ring entries is not virtio-pci property. > > It's a ring feature and the ring implementation is currently in the > generic virtio code. Ring features really have no home today so > virtio-pci seems logical. Why is this logical? Let's put the feature in generic virtio code, where ring itself it. That's what my patch does. >> And ev >> with virtio-pci properies, such as MSI, specific device should >> have control over number of vectors. >> > > Devices, or instantiation of the devices? The later is what I'm suggesting. The former would be my guess. Hard to see why different instances of blk would behave much differently. > Let's say we supported virtio-vbus along with virtio-pci. What does > virtio_blk_get_features() do to mask out sg_indirect? For all > virtio-blk knows, it could be on top of virtio-vbus. So? VIRTIO_RING_F_INDIRECT_DESC applies to all transports. Just clear this bit. >> Me as a user? We can't expect the user to tweak such low level stuff for >> each device. So devices such as block should have a say in which ring >> format options are used, in a way optimal for the specific usage. My >> example is that virtio net has merged buffers so indirect ring is >> probably just useless. And again, pci seems to have nothing to do with >> it, so why drag it in? >> > > If you want to tweak such thing, why not use default property values for > virtio-blk-pci's definition in virtio-pci.c? That keeps it out of > virtio-blk.c. Me as a user? I don't want to know what it is, much less tweak it. Me as a developer? Using different ring formats depending on transport is possible, but creates extra testing/benchmarking burden, reduces coverage. >>> separate qdev device than virtio-net-pci. It can have an identical >>> guest interface but within qemu, it should be a separate device. >>> This is how we handle the in-kernel PIT and it's how we should >>> handle the in-kernel APIC. >>> >> >> Ugh. What advantages does this have? > > It keeps a clean separate of the two devices. It actually ends up > making things a lot easier to understand because it's clear what > portions of code are not being used for the in-kernel device models. Ah, I see what you mean now. That separation is a wish I can't grant, sorry. See below. >> This would break things like >> migrating between userspace and kernel virtio (something that I >> support). > > The PIT uses a common state structure and common code for save/restore. > This makes migration compatible. Isn't device name put in the machine config, which presumably is send along as well? >> IMO, this should work like MSI, detect capability and just >> have virtio go faster, with a disable flag for troubleshooting purposes. >> >> Can migration between in-kernel and userspace PIT work? >> If yes we would be better off changing that, as well. >> > > Take a look at i8524{,-kvm.c} in qemu-kvm and how it's instantiated in > pc.c. It ends up being really straight forward. > >> Separate devices should be for things that have guest-visible >> differences. Don't try to encode random information into the device >> name. >> > > In this case, it's two separate implementations of the same device. I > think it makes sense for them to be separate devices. > > Regards, > > Anthony Liguori Hmm, I see what you mean. But kernel virtio is harder. Unlike PIT/APIC, it is not a separate codepath. It still needs all of userspace virtio to support live migration and non-MSI guests. Really, it's the same device that switches between kernel and userspace modes on the fly. This will become clearer from code when I implement migration for vhost, but basically you switch to userspace when you start migration, and back to kernel if migration fails. You also switch to kernel when MSI is enabled and back to userspace when it is disabled.
Michael S. Tsirkin wrote: >> Let's say we supported virtio-vbus along with virtio-pci. What does >> virtio_blk_get_features() do to mask out sg_indirect? For all >> virtio-blk knows, it could be on top of virtio-vbus. >> > > So? VIRTIO_RING_F_INDIRECT_DESC applies to all transports. > Just clear this bit. > You can have many layers with virtio. device + transport + ring virtio-vbus would have a different transport and a different ring implementation. So no, VIRTIO_RING_F_INDIRECT_DESC wouldn't apply to virtio-vbus. >>> This would break things like >>> migrating between userspace and kernel virtio (something that I >>> support). >>> >> The PIT uses a common state structure and common code for save/restore. >> This makes migration compatible. >> > > Isn't device name put in the machine config, which presumably is > send along as well? > Good question. I don't know the best way to resolve this. Maybe migration between devices isn't such a good idea. It's conceivable that vhost will require some state that isn't present in the userspace virtio-net. I think this requires some thought. >> In this case, it's two separate implementations of the same device. I >> think it makes sense for them to be separate devices. >> >> Regards, >> >> Anthony Liguori >> > > Hmm, I see what you mean. But kernel virtio is harder. Unlike > PIT/APIC, it is not a separate codepath. It still needs > all of userspace virtio to support live migration and non-MSI guests. > Really, it's the same device that switches between kernel and userspace > modes on the fly. > > This will become clearer from code when I implement migration for vhost, > but basically you switch to userspace when you start migration, and > back to kernel if migration fails. You also switch to kernel when MSI > is enabled and back to userspace when it is disabled. > Why bother switching to userspace for migration? Can't you just have get/set ioctls for the state? Regards, Anthony Liguori
On Tue, Aug 11, 2009 at 11:08:32AM -0500, Anthony Liguori wrote: > Michael S. Tsirkin wrote: >>> Let's say we supported virtio-vbus along with virtio-pci. What does >>> virtio_blk_get_features() do to mask out sg_indirect? For all >>> virtio-blk knows, it could be on top of virtio-vbus. >>> >> >> So? VIRTIO_RING_F_INDIRECT_DESC applies to all transports. >> Just clear this bit. >> > > You can have many layers with virtio. device + transport + ring > > virtio-vbus would have a different transport and a different ring > implementation. So no, VIRTIO_RING_F_INDIRECT_DESC wouldn't apply to > virtio-vbus. > >>>> This would break things like >>>> migrating between userspace and kernel virtio (something that I >>>> support). >>>> >>> The PIT uses a common state structure and common code for >>> save/restore. This makes migration compatible. >>> >> >> Isn't device name put in the machine config, which presumably is >> send along as well? >> > > Good question. I don't know the best way to resolve this. > > Maybe migration between devices isn't such a good idea. It's > conceivable that vhost will require some state that isn't present in the > userspace virtio-net. It can't. It switches to userspace before migration. > I think this requires some thought. > >>> In this case, it's two separate implementations of the same device. >>> I think it makes sense for them to be separate devices. >>> >>> Regards, >>> >>> Anthony Liguori >>> >> >> Hmm, I see what you mean. But kernel virtio is harder. Unlike >> PIT/APIC, it is not a separate codepath. It still needs >> all of userspace virtio to support live migration and non-MSI guests. >> Really, it's the same device that switches between kernel and userspace >> modes on the fly. >> >> This will become clearer from code when I implement migration for vhost, >> but basically you switch to userspace when you start migration, and >> back to kernel if migration fails. You also switch to kernel when MSI >> is enabled and back to userspace when it is disabled. >> > > Why bother switching to userspace for migration? Can't you just have > get/set ioctls for the state? I have these. But live migration requires dirty page logging. I do not want to implement it in kernel. > Regards, > > Anthony Liguori >
Michael S. Tsirkin wrote: > On > >> Why bother switching to userspace for migration? Can't you just have >> get/set ioctls for the state? >> > > I have these. But live migration requires dirty page logging. > I do not want to implement it in kernel. > Is it really that difficult? I think it would be better to just do that. I wonder though if mmu notifiers can be used to make it transparent... Regards, Anthony Liguori >> Regards, >> >> Anthony Liguori >> >>
On Wed, Aug 12, 2009 at 02:18:20PM -0500, Anthony Liguori wrote: > Michael S. Tsirkin wrote: >> On >> >>> Why bother switching to userspace for migration? Can't you just have >>> get/set ioctls for the state? >>> >> >> I have these. But live migration requires dirty page logging. >> I do not want to implement it in kernel. >> > > Is it really that difficult? I think it would be better to just do that. The idea is if it can be in userspace, let's keep it there. > I wonder though if mmu notifiers can be used to make it transparent... Maybe they can, but that decision belongs to KVM. Avi, what do you think? > Regards, > > Anthony Liguori > >>> Regards, >>> >>> Anthony Liguori >>> >>>
On 08/13/2009 09:15 AM, Michael S. Tsirkin wrote: > >> I wonder though if mmu notifiers can be used to make it transparent... >> > > Maybe they can, but that decision belongs to KVM. > Avi, what do you think? > > I don't see how mmu notifiers help. You can use mmu notifiers to sync an external mmu to the linux pagetables, but that's not the case here. I see the following options: - mprotect() guest memory, trap faults, mprotect() back Will work, but exceedingly slowly and wastefully - add a generic user visible dirty bit tracking based on linux ptes A lot of work, not sure if useful beyond kvm - implement vhost dirty bit tracking Not too difficult; not sure if it's worth the effort though - reuse the kvm dirty bitmap Not too difficult but introduces tight coupling between vhost and kvm which might slow down development - drop to userspace for live migration Reuse qemu code, lose some performance
On Thu, Aug 13, 2009 at 12:28:52PM +0300, Avi Kivity wrote: > On 08/13/2009 09:15 AM, Michael S. Tsirkin wrote: >> >>> I wonder though if mmu notifiers can be used to make it transparent... >>> >> >> Maybe they can, but that decision belongs to KVM. >> Avi, what do you think? >> >> > > I don't see how mmu notifiers help. You can use mmu notifiers to sync > an external mmu to the linux pagetables, but that's not the case here. > > I see the following options: > > - mprotect() guest memory, trap faults, mprotect() back > > Will work, but exceedingly slowly and wastefully I think this is what Anthony had in mind. > - add a generic user visible dirty bit tracking based on linux ptes > > A lot of work, not sure if useful beyond kvm > > - implement vhost dirty bit tracking > > Not too difficult; not sure if it's worth the effort though > > - reuse the kvm dirty bitmap > > Not too difficult but introduces tight coupling between vhost and kvm > which might slow down development > > - drop to userspace for live migration > > Reuse qemu code, lose some performance This is what I planned. Note that ability to drop to userspace is required for non-MSI mode, anyway. > -- > error compiling committee.c: too many arguments to function
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c index 7ca783e..15b50bb 100644 --- a/hw/virtio-balloon.c +++ b/hw/virtio-balloon.c @@ -127,7 +127,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev, static uint32_t virtio_balloon_get_features(VirtIODevice *vdev) { - return 0; + return virtio_common_features(); } static ram_addr_t virtio_balloon_to_target(void *opaque, ram_addr_t target) diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index 2beff52..7d33fee 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -364,7 +364,7 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev) if (strcmp(s->serial_str, "0")) features |= 1 << VIRTIO_BLK_F_IDENTIFY; - return features; + return features | virtio_common_features(); } static void virtio_blk_save(QEMUFile *f, void *opaque) diff --git a/hw/virtio-console.c b/hw/virtio-console.c index 663c8b9..ac25499 100644 --- a/hw/virtio-console.c +++ b/hw/virtio-console.c @@ -53,7 +53,7 @@ static void virtio_console_handle_input(VirtIODevice *vdev, VirtQueue *vq) static uint32_t virtio_console_get_features(VirtIODevice *vdev) { - return 0; + return virtio_common_features(); } static int vcon_can_read(void *opaque) diff --git a/hw/virtio-net.c b/hw/virtio-net.c index ef9e7ff..2e51a6a 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -154,7 +154,7 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev) } #endif - return features; + return features | virtio_common_features(); } static uint32_t virtio_net_bad_features(VirtIODevice *vdev) diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 3b9bfd1..90f51be 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -232,9 +232,6 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr) switch (addr) { case VIRTIO_PCI_HOST_FEATURES: ret = vdev->get_features(vdev); - ret |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY); - ret |= (1 << VIRTIO_RING_F_INDIRECT_DESC); - ret |= (1 << VIRTIO_F_BAD_FEATURE); break; case VIRTIO_PCI_GUEST_FEATURES: ret = vdev->features; diff --git a/hw/virtio.h b/hw/virtio.h index aa55677..de620a7 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -166,4 +166,14 @@ VirtIODevice *virtio_net_init(DeviceState *dev); VirtIODevice *virtio_console_init(DeviceState *dev); VirtIODevice *virtio_balloon_init(DeviceState *dev); +static inline uint32_t virtio_common_features(void) +{ + uint32_t features = 0; + features |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY); + features |= (1 << VIRTIO_RING_F_INDIRECT_DESC); + features |= (1 << VIRTIO_F_BAD_FEATURE); + + return features; +} + #endif
devices should have the final say over which virtio features they support. E.g. indirect entries may or may not make sense in the context of virtio-console. Move the common bits from virtio-pci to an inline function and let each device call it. No functional changes. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- This is part of my vhost work, but IMO the patch makes sense separately as well: the common features are not related to pci at all. hw/virtio-balloon.c | 2 +- hw/virtio-blk.c | 2 +- hw/virtio-console.c | 2 +- hw/virtio-net.c | 2 +- hw/virtio-pci.c | 3 --- hw/virtio.h | 10 ++++++++++ 6 files changed, 14 insertions(+), 7 deletions(-)